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

Public Shared Polls queries for users in a loop? #2855

Closed
7 of 18 tasks
b90g opened this issue Apr 13, 2023 · 14 comments · Fixed by #3091
Closed
7 of 18 tasks

Public Shared Polls queries for users in a loop? #2855

b90g opened this issue Apr 13, 2023 · 14 comments · Fixed by #3091
Milestone

Comments

@b90g
Copy link

b90g commented Apr 13, 2023

⚠️ This issue respects the following points: ⚠️

  • This is a bug, not a question or a configuration/webserver/proxy issue.
  • I agree to follow Nextcloud's Code of Conduct.
  • This issue is not already reported on Github (I've searched it).

What went wrong, what did you observe?

When i share a poll publicly and enter a user name the interfaces queries for existing user names until it breaks. in my browser meanwhile i get a green check mark that i can use that user name. when i submit my user name to start polling the dialog windows stays afloat the poll and i can not enter my polling options.

What did you expect, how polls should behave instead?

  1. I open the link.
  2. I enter a name so people know who submitted their poll options
  3. I submit my polls
    fin

What steps does it need to replay this bug?

1.Publicly share a poll (see first paragraph)

Installation method

Installed/updated from the appstore (Apps section of your site)

Installation type

First time installation

Affected polls version

5.0.0

Which browser did you use, when experiencing the bug?

  • Firefox
  • Chrome
  • Chromium/Chromium based (i.e. Edge)
  • Safari
  • Other/Don't know

Other browser

(112 versions on both browsers, no plugins installed)

Which System did you use, when experiencing the bug?

  • Windows
  • Linux
  • MacOs
  • iOS
  • Android
  • Other/Don't know

Other OS

No response

Add your browser log here

$ is deprecated: The global jQuery is deprecated. It will be removed in a later versions without another warning. Please ship your own.

Additional client environment information

No response

NC version

Nextcloud 25 (Nextcloud Hub 3)

Other Nextcloud version

No response

PHP engine version

PHP 8.1

Other PHP version

No response

Database engine

MariaDB

Database Engine version or other Database

10.5.18

Which user-backends are you using?

  • Default user-backend (database)
  • LDAP/ Active Directory
  • SSO - SAML
  • Other/Don't know

Add your nextcloud server log here

after entering the user name:


{"reqId":"ZqGyJX77D1ZOKAulKMic","level":0,"time":"2023-04-13T11:49:00+02:00","remoteAddr":"2a03:d9c0:3000::a21e","user":"--","app":"polls","method":"POST","url":"/apps/polls/check/username","message":"Controller OCA\\Polls\\Controller\\PublicController::validatePublicUsername created 54746 QueryBuilder objects, please check if they are created inside a loop by accident.","userAgent":"Mozilla/5.0 (Windows NT 10.0; rv:112.0) Gecko/20100101 Firefox/112.0","version":"25.0.5.1","data":{"app":"polls"}}
{"reqId":"ZqGyJX77D1ZOKAulKMic","level":2,"time":"2023-04-13T11:49:00+02:00","remoteAddr":"2a03:d9c0:3000::a21e","user":"--","app":"polls","method":"POST","url":"/apps/polls/check/username","message":"Controller OCA\\Polls\\Controller\\PublicController::validatePublicUsername executed 54746 queries.","userAgent":"Mozilla/5.0 (Windows NT 10.0; rv:112.0) Gecko/20100101 Firefox/112.0","version":"25.0.5.1","data":{"app":"polls"}}

when submitting the user name:

{"reqId":"Iwaeri5OxQtk8SWG0YWp","level":2,"time":"2023-04-13T11:55:01+02:00","remoteAddr":"2a03:d9c0:3000::a21e","user":"--","app":"no app in context","method":"POST","url":"/apps/polls/s/P3gbfNAX2Y9oZwgr/register?time=1681379700904","message":"Failed to parse mimetypemapping.json: Syntax error","userAgent":"Mozilla/5.0 (Windows NT 10.0; rv:112.0) Gecko/20100101 Firefox/112.0","version":"25.0.5.1","data":[]}
{"reqId":"Iwaeri5OxQtk8SWG0YWp","level":0,"time":"2023-04-13T11:55:01+02:00","remoteAddr":"2a03:d9c0:3000::a21e","user":"--","app":"user_saml","method":"POST","url":"/apps/polls/s/P3gbfNAX2Y9oZwgr/register?time=1681379700904","message":"/appinfo/app.php is deprecated, use \\OCP\\AppFramework\\Bootstrap\\IBootstrap on the application class instead.","userAgent":"Mozilla/5.0 (Windows NT 10.0; rv:112.0) Gecko/20100101 Firefox/112.0","version":"25.0.5.1","data":{"app":"user_saml"}}


### Additional environment informations

_No response_

### Configuration report

```shell
{
    "system": {
        "instanceid": "***REMOVED SENSITIVE VALUE***",
        "passwordsalt": "***REMOVED SENSITIVE VALUE***",
        "secret": "***REMOVED SENSITIVE VALUE***",
        "trusted_domains": [
            "nextcloud01.wolke-dev.verdigado.net",
            "wolke.dev.netzbegruenung.de"
        ],
        "datadirectory": "***REMOVED SENSITIVE VALUE***",
        "filesystem_check_changes": 0,
        "debug": true,
        "overwrite.cli.url": "https:\/\/wolke.dev.netzbegruenung.de",
        "dbtype": "mysql",
        "version": "25.0.5.1",
        "dbname": "***REMOVED SENSITIVE VALUE***",
        "dbhost": "***REMOVED SENSITIVE VALUE***",
        "dbport": "3306",
        "dbtableprefix": "oc_",
        "mysql.utf8mb4": true,
        "dbuser": "***REMOVED SENSITIVE VALUE***",
        "dbpassword": "***REMOVED SENSITIVE VALUE***",
        "installed": true,
        "mail_smtpmode": "smtp",
        "mail_smtpauthtype": "LOGIN",
        "mail_domain": "***REMOVED SENSITIVE VALUE***",
        "maintenance": false,
        "loglevel": 0,
        "logfile": "\/var\/log\/nextcloud.log",
        "logtimezone": "Europe\/Berlin",
        "theme": "",
        "filelocking.ttl": 3600,
        "filelocking.enabled": true,
        "redis": {
            "host": "***REMOVED SENSITIVE VALUE***",
            "port": 0,
            "dbindex": 0,
            "timeout": 1.5
        },
        "memcache.local": "\\OC\\Memcache\\Redis",
        "memcache.locking": "\\OC\\Memcache\\Redis",
        "memcache.distributed": "\\OC\\Memcache\\Redis",
        "updater.release.channel": "stable",
        "app_install_overwrite": [
            "files_readmemd",
            "onlyoffice"
        ],
        "preview_max_x": "2048",
        "preview_max_y": "2048",
        "data-fingerprint": "490ffc7bacdce6eb676bcbc2013215ca",
        "integrity.check.disabled": false,
        "simpleSignUpLink.shown": false,
        "log.condition": {
            "apps": [
                "admin_audit"
            ]
        },
        "maintenance_window_start": 1,
        "trusted_proxies": "***REMOVED SENSITIVE VALUE***"
    }
}

List of activated Apps

Enabled:
  - activity: 2.17.0
  - admin_audit: 1.15.0
  - bruteforcesettings: 2.5.0
  - calendar: 4.3.2
  - circles: 25.0.0
  - cloud_federation_api: 1.8.0
  - comments: 1.15.0
  - contacts: 5.2.0
  - contactsinteraction: 1.6.0
  - dav: 1.24.0
  - deck: 1.8.3
  - federatedfilesharing: 1.15.0
  - files: 1.20.1
  - files_antivirus: 4.0.3
  - files_pdfviewer: 2.6.0
  - files_sharing: 1.17.0
  - files_trashbin: 1.15.0
  - files_versions: 1.18.0
  - firstrunwizard: 2.14.0
  - groupfolders: 13.1.2
  - gruenegroupfoldermanagement: 2.0.0
  - keeweb: 0.6.12
  - logreader: 2.10.0
  - lookup_server_connector: 1.13.0
  - nextcloud_announcements: 1.14.0
  - notifications: 2.13.1
  - oauth2: 1.13.0
  - onlyoffice: 7.8.0
  - password_policy: 1.15.0
  - passwords: 2023.4.30
  - polls: 5.0.0
  - privacy: 1.9.0
  - provisioning_api: 1.15.0
  - ransomware_protection: 1.14.0
  - related_resources: 1.0.4
  - serverinfo: 1.15.0
  - settings: 1.7.0
  - sharebymail: 1.15.0
  - systemtags: 1.15.0
  - tasks: 0.14.5
  - text: 3.6.0
  - theming: 2.0.1
  - twofactor_backupcodes: 1.14.0
  - updatenotification: 1.15.0
  - user_saml: 6.0.0
  - viewer: 1.9.0
  - workflowengine: 2.7.0
Disabled:
  - apporder: 0.15.0
  - dashboard: 6.0.0
  - drawio: 1.0.3
  - encryption
  - federation: 1.5.0
  - files_external
  - files_rightclick: 0.15.1
  - impersonate: 1.10.0
  - photos: 1.4.0
  - recommendations: 0.6.0
  - support: 1.0.0
  - survey_client: 1.3.0
  - suspicious_login
  - twofactor_totp: 4.1.0
  - user_ldap
  - user_status: 1.2.0
  - weather_status: 1.2.0

Nextcloud Signing status

No errors have been found.

Additional Information

I use polls on a compare able setup but with ldap on its backend.

(dont mind the unredacted IP addresses, its a vpn anyway.)

@b90g b90g added the bug label Apr 13, 2023
@dartcafe
Copy link
Collaborator

I have no idea what this can be. Reproducing failed. But I have no SAML or SSO in any way.
I can see no loop, which could cause this error. Maybe the maintainers of the SSO app can enlighten me.

@github-actions
Copy link

This issue is marked as stale, because it had no activity in the last 30 days. It will be closed in 5 days.

@github-actions github-actions bot added the stale label Jul 31, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Aug 6, 2023
@b90g
Copy link
Author

b90g commented Aug 14, 2023

the issue still persists, thank you stale bot for considering my vacation time...

i am not a maintainer of a SSO App so i cant give you much feedback on this. should i contact them for you? open an issue there when you are sure that this is causing the user search in polls to fail? @dartcafe

@dartcafe
Copy link
Collaborator

And don't blame the stale bot. He just did his work. If you were on vacation and come back, just reopen the issue with a comment.

But I can't still not help because I have no idea, how to debug this issue or even reproduce it. I was only speculating, if another developer has a hint what is going wrong here.

The user check simply calls a search function from the core system and then the error is raised with your configuration.

To be precise, it is raised here: https://github.com/nextcloud/server/blob/2b7d03778fb47908174a3a2f380278addc98cae5/lib/private/AppFramework/Http/Dispatcher.php#L144-L159

@dartcafe dartcafe reopened this Aug 14, 2023
@dartcafe dartcafe removed the stale label Aug 14, 2023
@svenseeberg
Copy link

svenseeberg commented Sep 6, 2023

It seems that this is an performance issue with a large user & group database. The request took 35s in my test.

{"reqId":"V0Yi55clF6KB7GfyTHBH","level":2,"time":"2023-09-06T11:16:44+02:00","remoteAddr":"REMOTE_IP","user":"--","app":"polls","method":"POST","url":"/apps/polls/check/username","message":"Controller OCA\\Polls\\Controller\\PublicController::validatePublicUsername executed 54752 queries.","userAgent":"Mozilla/5.0 (X11; Linux x86_64; rv:109.0) Gecko/20100101 Firefox/117.0","version":"25.0.9.2","data":{"app":"polls"}}

The part PublicController::validatePublicUsername executed 54752 queries seems a bit excessive to me.

@dartcafe
Copy link
Collaborator

dartcafe commented Sep 6, 2023

@nickvergessen Any idea? Why are such numbers of queries created. I would expect it is just one query.

The error stated above is raised here, I assume:
https://github.com/nextcloud/server/blob/0c64e313b8c350b5872e107b0e4e1ab3d9767c0d/lib/private/AppFramework/Http/Dispatcher.php#L152-L159

For example, the search for existing usernames with IUserManager:search() is initiated here:

public static function search(string $query = '', array $skip = []): array {
$users = [];
foreach (Container::queryClass(IUserManager::class)->search($query) as $user) {
if (!in_array($user->getUID(), $skip)) {
$users[] = new self($user->getUID());
}
}
return $users;
}
}

A little bit context: Existing usernames are not allowed for participant's names in a public polls. For this a search for the requested (public) username is initiated.

The complete search method (also for groups i.e.) can be found here:

public function validatePublicUsername(string $userName, Share $share): bool {
if (!$userName) {
throw new TooShortException('Username must not be empty');
}
if ($share->getDisplayName() === $userName) {
return true;
}
$userName = strtolower(trim($userName));
// reserved usernames
if (str_contains($userName, 'deleted user') || str_contains($userName, 'anonymous')) {
throw new InvalidUsernameException;
}
// get all groups
foreach (Group::search() as $group) {
if ($userName === strtolower(trim($group->getId()))
|| $userName === strtolower(trim($group->getDisplayName()))) {
throw new InvalidUsernameException;
}
}
// get all users
foreach (User::search($userName) as $user) {
if ($userName === strtolower(trim($user->getId()))
|| $userName === strtolower(trim($user->getDisplayName()))) {
throw new InvalidUsernameException;
}
}
// get all participants
foreach ($this->voteMapper->findParticipantsByPoll($share->getPollId()) as $vote) {
if ($vote->getUserId()) {
if ($userName === strtolower(trim($vote->getUserId()))) {
throw new InvalidUsernameException;
}
}
}
// get all shares for this poll
foreach ($this->shareMapper->findByPoll($share->getPollId()) as $share) {
if ($share->getUserId() && $share->getType() !== Circle::TYPE) {
if ($userName === strtolower(trim($share->getUserId()))
|| $userName === strtolower(trim($share->getDisplayName()))) {
throw new InvalidUsernameException;
}
}
}
// return true, if username is allowed
return true;
}
}

@svenseeberg
Copy link

svenseeberg commented Sep 8, 2023

I found the issue. It comes from a custom group extension. However, I'm wondering if we need to fix something on both ends. First, obviously, we have an performance issue with our group back end.

But I'm also wondering why the groups should checked. Can groups vote? Is there a chance of mixing things up? Because I can vote in a publicly shared poll by entering a name "Test" and then later create a group named "Test". This worked without any issues. In the end this function is only leaking information about existing user and group names, isn't it? As far as I can tell I would simply drop the comparison and use the use input as is.

@dartcafe
Copy link
Collaborator

dartcafe commented Sep 9, 2023

Group names are reserved names of the site and therefore forbidden.

@svenseeberg
Copy link

svenseeberg commented Sep 10, 2023

Group names are reserved names of the site and therefore forbidden.

What happens if a user participates as "Test" and later a group "Test" is created? Nothing is preventing this right now. IMHO this results in undefined behavior.

I think it would be better to totally isolate poll participant display names from user & group names.

@dartcafe
Copy link
Collaborator

Yes, right, but at the moment it is as it is. Needs some massive change for that.

@svenseeberg
Copy link

Yes, right, but at the moment it is as it is. Needs some massive change for that.

Alright, then this issue can probably be closed.

@dartcafe
Copy link
Collaborator

Not sure, if the initial problem is solved. I was just regarding to the group name check.

@svenseeberg
Copy link

svenseeberg commented Sep 25, 2023

The initial problem is not a problem of the Polls app but of an internal app (that provides an additional group back end), that is not really as fast as it should be. IMHO the issue could be kept open as a reminder that things do not yet add up (for example that it is not allowed to name a polls participants as a group, but adding groups with names of polls participants is okay). But if you (the developers) do not consider this relevant, then it can be closed.

Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants