nextcloud / server

☁️ Nextcloud server, a safe home for all your data
https://nextcloud.com
GNU Affero General Public License v3.0
26.83k stars 4.01k forks source link

Autocomplete for LDAP Groups containing a dash ("-") faulty - most likely related to how LDAP backend searches/filters groups #15224

Closed fwolfst closed 1 year ago

fwolfst commented 5 years ago

Steps to reproduce

I guess the issue has nothing to do with sharing or autocomplete but with the ldap group backend and specifically how it is talked to when searching for (sub-)strings and how it constructs the filter. I am very willing to dive into that, if somebody gives me a pointer where the entry point in the code lies (I looked around in server/apps/user_ldap/lib/Group_LDAP.php etc. but am unsure where the string entered into the browsers text field will be consumed first).

Expected behaviour

The autocomplete should propose both (local and ldap) group that contain a dash in its group name.

Actual behaviour

The autocomplete stuff only shows the "local" group. Autocompletion does work however for the letters in front of the dash ("-"), so typing "my" in the share dialog will correctly show both groups.

Server configuration detail

Operating system: Linux 4.4.0-141-generic #167-Ubuntu SMP Wed Dec 5 10:40:15 UTC 2018 x86_64

Webserver: Apache/2.4.18 (Ubuntu) (fpm-fcgi)

Database: mysql 10.0.38

PHP version:

7.2.17-1+ubuntu16.04.1+deb.sury.org+3 Modules loaded: Core, date, libxml, openssl, pcre, zlib, filter, hash, Reflection, SPL, sodium, session, standard, cgi-fcgi, mysqlnd, PDO, xml, apcu, calendar, ctype, curl, dom, mbstring, fileinfo, ftp, gd, gettext, iconv, igbinary, imagick, intl, json, ldap, exif, mysqli, pdo_mysql, Phar, posix, readline, redis, shmop, SimpleXML, smbclient, sockets, sysvmsg, sysvsem, sysvshm, tokenizer, wddx, xmlreader, xmlwriter, xsl, zip, libsmbclient, Zend OPcache

Nextcloud version: 15.0.7 - 15.0.7.0

Updated from an older Nextcloud/ownCloud or fresh install: update

Where did you install Nextcloud from: unknown

Signing status
List of activated apps ``` Enabled: - accessibility: 1.1.0 - activity: 2.8.2 - admin_audit: 1.5.0 - announcementcenter: 3.4.1 - audioplayer: 2.7.0 - calendar: 1.6.5 - cloud_federation_api: 0.1.0 - comments: 1.5.0 - contacts: 3.1.1 - dav: 1.8.1 - drawio: 0.9.2 - external: 3.2.0 - federatedfilesharing: 1.5.0 - federation: 1.5.0 - files: 1.10.0 - files_accesscontrol: 1.5.0 - files_automatedtagging: 1.5.0 - files_downloadactivity: 1.4.0 - files_external: 1.6.0 - files_markdown: 2.0.6 - files_pdfviewer: 1.4.0 - files_readmemd: 1.0.2 - files_retention: 1.4.2 - files_rightclick: 0.13.0 - files_sharing: 1.7.0 - files_texteditor: 2.7.0 - files_trackdownloads: 1.4.0 - files_trashbin: 1.5.0 - files_versions: 1.8.0 - files_videoplayer: 1.4.0 - gallery: 18.2.0 - group_everyone: 0.1.2 - groupfolders: 3.0.0 - impersonate: 1.2.0 - issuetemplate: 0.5.0 - logreader: 2.0.0 - lookup_server_connector: 1.3.0 - nextcloud_announcements: 1.4.0 - notifications: 2.3.0 - oauth2: 1.3.0 - password_policy: 1.5.0 - polls: 0.10.2 - provisioning_api: 1.5.0 - richdocuments: 3.2.4 - serverinfo: 1.5.0 - sharebymail: 1.5.0 - spreed: 5.0.3 - systemtags: 1.5.0 - tasks: 0.9.8 - theming: 1.6.0 - twofactor_backupcodes: 1.4.1 - updatenotification: 1.5.0 - user_ldap: 1.5.0 - workflowengine: 1.5.0 Disabled: - bookmarks - bruteforcesettings - circles - encryption - firstrunwizard - gpxedit - mindmaps - music - rainloop - support - survey_client - user_usage_report ```
Configuration (config/config.php) ``` { "instanceid": "***REMOVED SENSITIVE VALUE***", "passwordsalt": "***REMOVED SENSITIVE VALUE***", "secret": "***REMOVED SENSITIVE VALUE***", "trusted_domains":"***REMOVED SENSITIVE VALUE***", "datadirectory": "***REMOVED SENSITIVE VALUE***", "overwrite.cli.url": "***REMOVED SENSITIVE VALUE***", "dbtype": "mysql", "version": "15.0.7.0", "dbname": "***REMOVED SENSITIVE VALUE***", "dbhost": "***REMOVED SENSITIVE VALUE***", "dbport": "", "dbtableprefix": "oc_", "dbuser": "***REMOVED SENSITIVE VALUE***", "dbpassword": "***REMOVED SENSITIVE VALUE***", "installed": true, "maintenance": false, "ldapIgnoreNamingRules": false, "ldapProviderFactory": "\\OCA\\User_LDAP\\LDAPProviderFactory", "mail_smtpmode": "smtp", "mail_smtpauthtype": "PLAIN", "mail_from_address": "***REMOVED SENSITIVE VALUE***", "mail_domain": "***REMOVED SENSITIVE VALUE***", "mail_smtpauth": 1, "mail_smtphost": "***REMOVED SENSITIVE VALUE***", "mail_smtpport": "25", "mail_smtpname": "***REMOVED SENSITIVE VALUE***", "mail_smtppassword": "***REMOVED SENSITIVE VALUE***", "theme": "", "loglevel": 2, "default_language": "de", "filelocking.enabled": "true", "memcache.local": "\\OC\\Memcache\\APCu", "memcache.locking": "\\OC\\Memcache\\Redis", "redis": { "host": "***REMOVED SENSITIVE VALUE***", "port": 0 }, "updater.release.channel": "stable" } ```

Are you using external storage, if yes which one: local/smb/sftp/...

Are you using encryption:

Are you using an external user-backend, if yes which one: LDAP (OpenLDAP)

LDAP configuration (delete this par if not used) ``` _lastChange: 1556174703background_sync_interval: 1800background_sync_offset: 0background_sync_prefix: cleanUpJobOffset: 50enabled: yeshas_memberof_filter_support: 1installed_version: 1.5.0ldap_agent_password: QUwtU3N0YmFnaS4=ldap_base: dc=REMOVED,dc=deldap_base_groups: ou=Groups,dc=REMOVED,dc=deldap_base_users: ou=People,dc=REMOVED,dc=deldap_cache_ttl: 600ldap_configuration_active: 1ldap_display_name: cnldap_dn: cn=admin,dc=REMOVED,dc=deldap_email_attr: mailldap_experienced_admin: 1ldap_group_filter: (&(|(objectclass=posixGroup)))ldap_group_member_assoc_attribute: memberUidldap_groupfilter_groups: ldap_groupfilter_objectclass: posixGroupldap_host: REMOVEDldap_login_filter: (&(&(|(objectclass=posixAccount)))(|(uid=%uid)))ldap_login_filter_mode: 1ldap_loginfilter_attributes: cnldap_port: 389ldap_tls: 0ldap_user_filter_mode: 0ldap_userfilter_groups: ldap_userfilter_objectclass: posixAccountldap_userlist_filter: (&(|(objectclass=posixAccount)))types: authentication ```

Client configuration

Browser: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:66.0) Gecko/20100101 Firefox/66.0

Operating system: Ubuntu

fwolfst commented 5 years ago

But maybe I am on the wrong track with looking for the ldap query filter, as all group names should be cached, isnt it? Like the autocomplete function of a share-dialog should only hit the cached group names and not fire a ldap query for every letter typed? If so, why is there a difference in behaviour between LDAP and "local" groups?

DagNygren commented 5 years ago

I see the same thing here with the same LDAP config. Something goes has gone wrong in an update during this spring and the wizard doesn't work. The LDAP part doesn't seem to be very much tested outside the AD domain... Anyway: All the logins using LDAP hangs (and creates zombies, which eventually kills nextcloud with "too many connections to the database") and the wizard is not able to list any groups but just stay there spinning the cursor. This worked fine earlier and there has been no changes in the LDAP setup. All the other programs using the LDAP server work fine. No dashes in my group names, though. My setup:

fwolfst commented 5 years ago

@DagNygren cool that you want to contribubte and sad to hear that you have some issues. However, I believe your issues are unrelated to this one (you don't have dashes in group names at all). So, you could open a new ticket and maybe even remove your comment here, to reduce message clutter. Chances that someone gets interest in your setup will get much higher when it has the right heading (at the moment, I'd like to talk about autocompletion and dashes in ldap group names here). If you open a new issue, maybe I can help (a tiny bit) looking into whats wrong.

blizzz commented 5 years ago

LDAP searches go live against the server (unless cached), so we can take advantage of the LDAP servers search capabilities.

Starting point to look for would be apps/user_ldap/lig/Group_LDAP.php, method search or searchGroups.

fwolfst commented 5 years ago

The general fix to allow "fulltext" search with LDAP (also for username and other entities) would be to change prepareSearchTerm, where the documentation reads:

 * returns the search term depending on whether we are allowed
 * list users found by ldap with the current input appended by
 * a *

(the last "*" is a literal "star" :star:).

The relevant code then is:

$result = $term . '*';

Imho this has to be changed to $result = '*' . $term . '*'; . I applied that fix on my production instance and did not yet notice any issues. Full text search in LDAP Group- and Usernames now works as expected (by me).

@blizzz I happily craft a PR, any chance it would get merged? Any background on why the LDAP filter is created only with a forward glob?

blizzz commented 5 years ago

Imho this has to be changed to $result = '' . $term . '';

Unconditional wildcards may cause really bad performance on slightly bigger setups as indexing is being circumvented. Actually, when you start your query with an asterisk (*), then it would run the query against LDAP with it. It's whitelisted in that case.

fwolfst commented 5 years ago

Actually, when you start your query with an asterisk (*), then it would run the query against LDAP with it. It's whitelisted in that case.

Can you elaborate on that? I am sorry but do not understand. As said, in my case the preprended asterisk seems to lead to the correct results.

I do understand that an asterisk will circumvent the indexing, and I guess "unconditional wildcards" means wildcards without context, or anchor - or do you mean that the query should be guarded, by checking whether term actually contains some characters? Or do you mean that if the first character is an asterisk the query is "whitelisted" as in: will not hit cache?

What would you propose to fix that behaviour? A setting that allows LDAP search to deliver "correct" results with the price of a potential performance hit? Add a info on the search field that some LDAP search is limited?

blizzz commented 5 years ago

@fwolfst i meant that there should not be a wildcard prepended by default. If you manually trigger a search with an asterisk up front, it will work, but given this is not really something for end users. A config flag to – if set – to insert the wildcard with every search would be easiest, yet another strange code path to consider. Then, i do not have better idea. Maybe to consider is, with this flag, to do a normal search first, and if this does not yield any result repeat the search with the wildcard. The problem with this is, if it yields an undesired match, nothing is won. And it would make it more complex.

fwolfst commented 5 years ago

Mmmh, I dont think I will find the time to look into how nextcloud handles configuration and provide a clean patch.

@blizzz : Do I understand correctly that you maintain this plugin? Would you merge a patch that applies the asterisk-search only on "manual" searches (where a user enters something in the text-box), as opposed to every internal search through LDAP? Do you think there would be an easy implementation for such behaviour (I did not yet understand the frontend code paths)?

Another option could be to fork the plugin ("SearcheableLDAP"), but I guess for now I will have to live with manual patches :( . Unfortunately, before finding the bug, I decided on a convention where certain teams have Groups names ending on -team. The performance penalty is absolutely no issue for me (~120 Users, a big dozen of groups maybe), and correct search behavior is super important, otherwise people do not manage to share their stuff correctly.

blizzz commented 5 years ago

@blizzz : Do I understand correctly that you maintain this plugin? Would you merge a patch that applies the asterisk-search only on "manual" searches (where a user enters something in the text-box), as opposed to every internal search through LDAP? Do you think there would be an easy implementation for such behaviour (I did not yet understand the frontend code paths)?

I am having my doubts that you can identify those spots. As, anything goes via the User backend API methods, getUsers mostly here. So you cannot really distinguish between internal and external uses. Further, I still would not want this to be the default behaviour.

fwolfst commented 4 years ago

Just sharing for people who might run into the same issue with their users (some ldap groups do not show up, although it seems that autocompletion does work in some cases):

The relevant change can be made with

sed -i "s/\$result = \$term . '\*';/\$result = '*' . \$term . '\*';/" apps/user_ldap/lib/Access.php

as of today, NC 16.0.6 .

szaimen commented 3 years ago

Is this Issue still valid? If not, please close this issue. Thanks! :)

fwolfst commented 3 years ago

Is this Issue still valid? If not, please close this issue. Thanks! :)

If you want the user search (e.g. in sharing dialog) to find usernames in ldap that do not begin with whatever you entered, but just contain it (e.g. 'net' finding 'Anette' and 'janet'), you still have to patch after each update. Imho not closed.

An option to enable the proper search would be an solution, but I have no time to dig into plugin options.

szaimen commented 1 year ago

Hi, please update to 24.0.8 or better 25.0.2 and report back if it fixes the issue. Thank you!