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

Sharing still broken with ldap groups #25062

Closed
christianvw opened this issue Jan 10, 2021 · 29 comments · Fixed by #40367
Closed

Sharing still broken with ldap groups #25062

christianvw opened this issue Jan 10, 2021 · 29 comments · Fixed by #40367
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap 26-feedback bug feature: ldap feature: sharing

Comments

@christianvw
Copy link

christianvw commented Jan 10, 2021

How to use GitHub

  • Please use the 👍 reaction to show that you are affected by the same issue.
  • Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
  • Subscribe to receive notifications on status change and new comments.

Steps to reproduce

  1. Login as administrator to your nextcloud
  2. Activate LDAP user and group backend app
  3. Configure your ldap settings in http://localhost/settings/admin/ldap
  4. Create a group in your ldap server, called sharing_group
  5. Create a folder in your nextcloud, called shared_folder
  6. Share the folder shared_folder with the group from ldap sharing_group
  7. Now (after sharing) create a user called sharing_user in your ldap server
  8. Add the created ldap user to the ldap group sharing_group
  9. Login to your nextcloud as the newly created user sharing_user

Expected behaviour

  • User sharing_user should be inside the ldap group sharing_group (can be seen by the user sharing_user in http://localhost/settings/user)
  • User sharing_user therefore should see the folder shared_folder since this is shared with the group sharing_group where the user is a member

Actual behaviour

  • ✔️ User sharing_user IS inside the ldap group sharing_group
  • ❌ User sharing_user CANNOT see the folder shared_folder

The bug: LDAP users created after the share creation are not able to view the share to their group (which they normally should be able to).

The problem / a workaround

The problem might be located in the sharing or ldap app. It could be that the share is not automatically accepted. Therefore I went into the database using docker exec -it <<<<containername-of-db>>>> mysql -u<<<<username-of-db>>>> -p <<<<name-of-database>>>> and displayed the table with all shares:

MariaDB [nextcloud]> SELECT * FROM oc_share;
+----+------------+--------------------------------------+----------+-----------+---------------+--------+-----------+-------------+-------------+-------------+----------------+-------------+------------+----------+------------+-------+-----------+------------+------------------+------+---------------+-------+
| id | share_type | share_with                           | password | uid_owner | uid_initiator | parent | item_type | item_source | item_target | file_source | file_target    | permissions | stime      | accepted | expiration | token | mail_send | share_name | password_by_talk | note | hide_download | label |
+----+------------+--------------------------------------+----------+-----------+---------------+--------+-----------+-------------+-------------+-------------+----------------+-------------+------------+----------+------------+-------+-----------+------------+------------------+------+---------------+-------+
|  1 |          1 | sharing_group                        | NULL     | root      | root          |   NULL | folder    | 89          | NULL        |          89 | /shared_folder |          31 | 1610308767 |        0 | NULL       | NULL  |         0 | NULL       |                0 | NULL |             0 | NULL  |
|  2 |          2 | 5d6f515c-e7c2-103a-9933-21e2415ffd4b | NULL     | root      | root          |      1 | folder    | 89          | NULL        |          89 | /shared_folder |          31 | 1610308767 |        1 | NULL       | NULL  |         0 | NULL       |                0 | NULL |             0 | NULL  |
|  3 |          2 | cc512224-e7c4-103a-9934-21e2415ffd4b | NULL     | root      | root          |      1 | folder    | 89          | NULL        |          89 | /shared_folder |          31 | 1610308767 |        1 | NULL       | NULL  |         0 | NULL       |                0 | NULL |             0 | NULL  |
+----+------------+--------------------------------------+----------+-----------+---------------+--------+-----------+-------------+-------------+-------------+-------------+-------------+------------+----------+------------+-------+-----------+------------+------------------+------+---------------+-------+

Now we notice that the group we actually shared the folder with, supposedly did not accept this share (see row with id = 1, see column accepted). Obviously a group cannot accept a share manually, however I would have assumed that this would happen automatically, with such system-level sharing targets.

Now I just tried to set accepted to 1 in the affected line. And indeed it worked. 🎉 Now all ldap users see the shared folder shared_folder since they are in the group sharing_group. This works especially for ldap users created after the share was created.

MariaDB [nextcloud]> UPDATE `oc_share` SET `accepted` = 1 WHERE `share_with` = 'sharing_group' AND `file_target` = '/shared_folder';
Query OK, 1 row affected (0.005 sec)
Rows matched: 1  Changed: 1  Warnings: 0

Therefore I would limit the bug to this area, that shares to ldap groups are not automatically accepted (in the database). Groups obviously cannot accept shares by itself since no one really is the group. Is this maybe the underlying bug?

Not working workarounds

There are many hints and workarounds posted here in many issues but none of them fixed the problem for me. Here is a list of the not working ones:

  • The user sharing_user can accept the share by itself on http://localhost/apps/files/?dir=/&view=pendingshares (the pending share is visible but clicking accept results in a 404 api request)
  • Starting cronjobs manually via docker exec -it --user www-data -it <<<<container-name>>>> php /var/www/html/cron.php (doesn't change anything)
  • The OCC Repair command docker exec -it --user www-data -it <<<<container-name>>>> php occ maintenance:repair (doesn't change anything)
  • Setting sharing.force_share_accept to true via docker exec --user www-data -it <<<<container-name>>>> php occ config:app:set sharing force_share_accept --value=true (doesn't change anything)

Possibly related issues

There are quite a few issues regarding this topic but unfortunately the same issue still exists for the current nextcloud version.

Server configuration

Operating system: Docker version 20.10.2, build 2291f61

Web server: Apache

Database: 10.5.8-MariaDB-1:10.5.8+maria~focal

PHP version: 7.4.14

Nextcloud version: 20.0.4

Updated from an older Nextcloud/ownCloud or fresh install: Fresh install, but also happends with updated instances (since Nextcloud 20)

Where did you install Nextcloud from: Docker Hub

Signing status:

Signing status
No errors have been found.

List of activated apps:

App list
Enabled:
  - accessibility: 1.6.0
  - activity: 2.13.4
  - bruteforcesettings: 2.0.1
  - cloud_federation_api: 1.3.0
  - comments: 1.10.0
  - contactsinteraction: 1.1.0
  - dashboard: 7.0.0
  - dav: 1.16.2
  - federatedfilesharing: 1.10.2
  - federation: 1.10.1
  - files: 1.15.0
  - files_pdfviewer: 2.0.1
  - files_rightclick: 0.17.0
  - files_sharing: 1.12.1
  - files_trashbin: 1.10.1
  - files_versions: 1.13.0
  - files_videoplayer: 1.9.0
  - firstrunwizard: 2.9.0
  - groupfolders: 8.2.0
  - logreader: 2.5.0
  - lookup_server_connector: 1.8.0
  - nextcloud_announcements: 1.9.0
  - notifications: 2.8.0
  - oauth2: 1.8.0
  - password_policy: 1.10.1
  - photos: 1.2.1
  - privacy: 1.4.0
  - provisioning_api: 1.10.0
  - recommendations: 0.8.0
  - serverinfo: 1.10.0
  - settings: 1.2.0
  - sharebymail: 1.10.0
  - support: 1.3.0
  - survey_client: 1.8.0
  - systemtags: 1.10.0
  - text: 3.1.0
  - theming: 1.11.0
  - twofactor_backupcodes: 1.9.0
  - updatenotification: 1.10.0
  - user_ldap: 1.10.2
  - user_status: 1.0.1
  - viewer: 1.4.0
  - weather_status: 1.0.0
  - workflowengine: 2.2.0
Disabled:
  - admin_audit
  - encryption
  - files_external

Nextcloud configuration:

Config report
{
    "system": {
        "htaccess.RewriteBase": "\/",
        "memcache.local": "\\OC\\Memcache\\APCu",
        "apps_paths": [
            {
                "path": "\/var\/www\/html\/apps",
                "url": "\/apps",
                "writable": false
            },
            {
                "path": "\/var\/www\/html\/custom_apps",
                "url": "\/custom_apps",
                "writable": true
            }
        ],
        "instanceid": "***REMOVED SENSITIVE VALUE***",
        "passwordsalt": "***REMOVED SENSITIVE VALUE***",
        "secret": "***REMOVED SENSITIVE VALUE***",
        "trusted_domains": [
            "localhost"
        ],
        "datadirectory": "***REMOVED SENSITIVE VALUE***",
        "dbtype": "mysql",
        "version": "20.0.4.0",
        "overwrite.cli.url": "http:\/\/localhost",
        "dbname": "***REMOVED SENSITIVE VALUE***",
        "dbhost": "***REMOVED SENSITIVE VALUE***",
        "dbport": "",
        "dbtableprefix": "oc_",
        "mysql.utf8mb4": true,
        "dbuser": "***REMOVED SENSITIVE VALUE***",
        "dbpassword": "***REMOVED SENSITIVE VALUE***",
        "installed": true,
        "ldapIgnoreNamingRules": false,
        "ldapProviderFactory": "OCA\\User_LDAP\\LDAPProviderFactory",
        "maintenance": false
    }
}

Are you using external storage, if yes which one: None

Are you using encryption: no

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

LDAP configuration (delete this part if not used)

LDAP config
+-------------------------------+-----------------------------------------------------------------------+
| Configuration                 | s01                                                                   |
+-------------------------------+-----------------------------------------------------------------------+
| hasMemberOfFilterSupport      |                                                                       |
| homeFolderNamingRule          |                                                                       |
| lastJpegPhotoLookup           | 0                                                                     |
| ldapAgentName                 | cn=query,dc=ldap,dc=example,dc=com                           |
| ldapAgentPassword             | ***                                                                   |
| ldapAttributesForGroupSearch  |                                                                       |
| ldapAttributesForUserSearch   |                                                                       |
| ldapBackupHost                |                                                                       |
| ldapBackupPort                |                                                                       |
| ldapBase                      | dc=ldap,dc=example,dc=com                                          |
| ldapBaseGroups                | ou=groups,dc=ldap,dc=example,dc=com                                |
| ldapBaseUsers                 | ou=people,dc=ldap,dc=example,dc=com                                |
| ldapCacheTTL                  | 600                                                                   |
| ldapConfigurationActive       | 1                                                                     |
| ldapDefaultPPolicyDN          |                                                                       |
| ldapDynamicGroupMemberURL     |                                                                       |
| ldapEmailAttribute            | mail                                                                  |
| ldapExperiencedAdmin          | 0                                                                     |
| ldapExpertUUIDGroupAttr       |                                                                       |
| ldapExpertUUIDUserAttr        |                                                                       |
| ldapExpertUsernameAttr        |                                                                       |
| ldapExtStorageHomeAttribute   |                                                                       |
| ldapGidNumber                 | gidNumber                                                             |
| ldapGroupDisplayName          | cn                                                                    |
| ldapGroupFilter               | (&(|(objectclass=posixGroup)))                                        |
| ldapGroupFilterGroups         |                                                                       |
| ldapGroupFilterMode           | 0                                                                     |
| ldapGroupFilterObjectclass    | posixGroup                                                            |
| ldapGroupMemberAssocAttr      | memberUid                                                             |
| ldapHost                      | production-ldap                                                       |
| ldapIgnoreNamingRules         |                                                                       |
| ldapLoginFilter               | (&(|(objectclass=inetOrgPerson)(objectclass=posixAccount))(uid=%uid)) |
| ldapLoginFilterAttributes     |                                                                       |
| ldapLoginFilterEmail          | 0                                                                     |
| ldapLoginFilterMode           | 0                                                                     |
| ldapLoginFilterUsername       | 1                                                                     |
| ldapMatchingRuleInChainState  | unknown                                                               |
| ldapNestedGroups              | 0                                                                     |
| ldapOverrideMainServer        |                                                                       |
| ldapPagingSize                | 500                                                                   |
| ldapPort                      | 389                                                                   |
| ldapQuotaAttribute            |                                                                       |
| ldapQuotaDefault              |                                                                       |
| ldapTLS                       | 0                                                                     |
| ldapUserAvatarRule            | default                                                               |
| ldapUserDisplayName           | cn                                                                    |
| ldapUserDisplayName2          |                                                                       |
| ldapUserFilter                | (|(objectclass=inetOrgPerson)(objectclass=posixAccount))              |
| ldapUserFilterGroups          |                                                                       |
| ldapUserFilterMode            | 0                                                                     |
| ldapUserFilterObjectclass     | inetOrgPerson;posixAccount                                            |
| ldapUuidGroupAttribute        | auto                                                                  |
| ldapUuidUserAttribute         | auto                                                                  |
| turnOffCertCheck              | 0                                                                     |
| turnOnPasswordChange          | 0                                                                     |
| useMemberOfToDetectMembership | 1                                                                     |
+-------------------------------+-----------------------------------------------------------------------+

Client configuration

Browser: Google Chrome 87.0.4280.141

Operating system: Windows 10 10.0.19042 Build 19042

Logs

Web server error log

Web server error log
Insert your webserver log here

Nextcloud log (data/nextcloud.log)

Nextcloud log
Insert your Nextcloud log here

Browser log

Browser log
No client sided problem
@christianvw christianvw added 0. Needs triage Pending check for reproducibility or if it fits our roadmap bug labels Jan 10, 2021
@larueli
Copy link

larueli commented Jan 24, 2021

We can see the same bug on our nextcloud instance for our university (Php 7.4, Nextcloud 20.0.5)

The solution provided above is working, thanks ! To quickly fix this issue on all group shared folders, we (with @jdorel) used the following SQL code :

UPDATE `oc_share` SET `accepted` = 1 WHERE `share_type` = 1;

accepted field for group folder shares should always be set to 1.

@faust64
Copy link
Contributor

faust64 commented Feb 4, 2021

Should it be? set to 1 when created?
AFAIU, there's a listener that's supposed to accept shares on behalf of users being added to groups, over there:

https://github.com/nextcloud/server/blob/master/apps/files_sharing/lib/Listener/UserAddedToGroupListener.php#L50

While it has been suggested ( #19520 (comment) ) that NextCloud cron should take care of these, I've been digging and could only find a job that removes shares to users that no longer belong to a group ( https://github.com/nextcloud/server/blob/master/lib/private/Repair/OldGroupMembershipShares.php#L69 ) . The event listener above seems to be the one accepting on behalf of users joining a group.

Then again, looking at the SELECT * FROM oc_share; reported above, it looks like users did accept it ....

If this is the case, we may then fix this in https://github.com/nextcloud/server/blob/master/lib/private/Share20/DefaultShareProvider.php#L149 , adding some $qb->setValue('accepted', $qb->createNamedParameter(IShare::STATUS_ACCEPTED)); in the IShare::TYPE_GROUP elseif from the create method. See master...Worteks:fix-sharewithgroup
Still, ... shouldn't we see similar issues reported, that wouldn't involve ldap groups? Patching there would change behavior regardless of the backend our group came from.

@christianvw could you confirm that cc512224-e7c4-103a-9934-21e2415ffd4b we see in the share_with column is indeed the user ID for the account you did add to your group, after the share was initially created?
Or were they two members in your group to begin with?

@coach1988
Copy link

@faust64

Still, ... shouldn't we see similar issues reported, that wouldn't involve ldap groups? Patching there would change behavior regardless of the backend our group came from.

I think there have been some without LDAP, e.g.

@gabriele-semino
Copy link

This issue seems to be pretty widespread. I have the same issue with the "Everyone" group, but just ran some tests and realized that it happens with all groups and users (all LDAPs in my case).

Here a similar issue reported for the "Everyone" group: icewind1991/group_everyone#19 (comment)

@daha96
Copy link

daha96 commented Apr 24, 2021

+1

@blizzz
Copy link
Member

blizzz commented Apr 30, 2021

Group memberships are checked in an hourly background job which sends the corresponding events.

@scroom
Copy link

scroom commented Apr 30, 2021

Group memberships are checked in an hourly background job which sends the corresponding events.

is there another workaround than the one described above (manual adjustment of the database)?

@blizzz
Copy link
Member

blizzz commented Apr 30, 2021

Group memberships are checked in an hourly background job which sends the corresponding events.

is there another workaround than the one described above (manual adjustment of the database)?

nope

@scroom
Copy link

scroom commented May 4, 2021

Group memberships are checked in an hourly background job which sends the corresponding events.

is there another workaround than the one described above (manual adjustment of the database)?

nope

Would it be problematic to change all entries for groups to accepted? And should not this be the default case for groups? Is this the actual bug as @christianvw mentioned?

Older group-shares have accepted set to 1. It might be a regression?

@blizzz
Copy link
Member

blizzz commented May 4, 2021

Group memberships are checked in an hourly background job which sends the corresponding events.

is there another workaround than the one described above (manual adjustment of the database)?

nope

Would it be problematic to change all entries for groups to accepted? And should not this be the default case for groups? Is this the actual bug as @christianvw mentioned?

Older group-shares have accepted set to 1. It might be a regression?

It depends on the configuration, but by default the shares are accepted. Still, sharing need to be made aware of group membership changes. I cannot reproduce any misbehaviour.

@pkolmann
Copy link

pkolmann commented Jun 7, 2021

Interestinly I see in my Nextcloud (21.0.2.1) installation the same isse.

In the oc_share table there is a row for every user who was in the LDAP group when sharing.
I tried adding the new user into this table as well and the share popped up right away for the new user as well.

Maybe it is intended to keep the accept=0 for the group and just a syncer is missing, who checks for new users and adds a line per new LDAP user and share?

@leuedaniel
Copy link

leuedaniel commented Jun 8, 2021

For me this trigger is the workaround:

CREATE DEFINER=root@localhostTRIGGERoc_group_user_after_insertAFTER INSERT ONoc_group_user FOR EACH ROW BEGIN if (SELECT COUNT(id) FROM oc_share WHERE share_with = NEW.gid and share_type = 1)>0 then INSERT INTO oc_share(share_type,share_with,uid_owner,uid_initiator,parent,item_type,item_source,file_source,file_target,permissions,stime,accepted,expiration,note) SELECT 2, NEW.uid, uid_owner, uid_initiator, id, item_type, item_source, file_source, file_target, permissions, stime, 1, expiration, note FROM oc_share WHERE share_with = NEW.gid AND share_type = 1; END if; END

but its better when we solve that in code.

There are two things, the shares for a group must have accepted = 1 and new users in the group must inherit the group sharing like in my trigger above.

I've tested it with the SQL backends app and it works with new users, i think it works also with LDAP.

@christianvw
Copy link
Author

christianvw commented Jun 18, 2021

Small recap of the problem

LDAP users created (and added to the ldap group) after the share creation in nextcloud are not getting the share (because it does not get accepted by nextcloud).

Problem still there, reproducibility

I still see exactly the same behavior in Nextcloud 21.0.2 (Docker), even in a fresh installation.

For me, this misbehavior is 100% reproducible with the process described in the initial post. Mind you with Nextcloud as well as openLDAP in a docker container.

And considering the many competitors in these and other issues, it is also a problem for at least some others. Even new issues popped up in the last months, regarding the exact same problem.

I wonder how this problem doesn't seem to occur in large organizations that use Nextcloud in combination with LDAP, this issue should stand out.

Fixes in the database

However, like @leuedaniel said, the true solution must be a fix in the Nextcloud application (a fix of the background job / syncer / group share creation / ...). Is there maybe even a fix coming? @blizzz

Possible (real) fixes in the application

@faust64 did a pull request (one liner, #26428) with the idea from my initial post in mind: Setting the accepted to 1 for group shares (directly on group share creation).

His change (in lib/private/Share20/DefaultShareProvider.php):

//...

} elseif ($share->getShareType() === IShare::TYPE_GROUP) {
    //Set the GID of the group we share with
    $qb->setValue('share_with', $qb->createNamedParameter($share->getSharedWith()));
    $qb->setValue('accepted', $qb->createNamedParameter(IShare::STATUS_ACCEPTED)); //<-- **NEW**
    
    //...

However @blizzz has been looking more at a background job. This would also be an alternative solution.

Nevertheless something does not work here as intended.

@szaimen szaimen added 1. to develop Accepted and waiting to be taken care of feature: ldap feature: sharing and removed 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Aug 8, 2021
@Gianni154
Copy link

Any updates? Issue also present in Nextcloud 23

@come-nc
Copy link
Contributor

come-nc commented Jan 3, 2022

Here is the behavior I get:

When creating a user and adding it to a group in ldap, indeed the share is not accepted automatically. It appears in pending shares, and I can accept it (no errors, accepting works).
When doing the same but in nextcloud, the share is accepted automatically. The problem is specific to LDAP.

But, if I wait until the background job runs, the share is correctly accepted.

So, this is not a big problem as of 23 from what I can see, it’s a matter of waiting a few hours top, or manually accepting the pending share. It is annoying but it is not a major issue.
If we want to fix this we need to find a way to trigger the UserAddedEvent event on the user for all groups it is a member of on first login, but we must be careful to avoid the background job emitting the event a second time, so it is not as trivial as it may sound.

@cybrwshl
Copy link

You're right manually accepting works for me. Didn't know there is a workflow to accept pending shares.

@schlagmichdoch
Copy link

It appears we have the very same problem with our Nextcloud 23.0.2 and openldap-2.4.40

Can someone please confirm that this throws log error messages like:

OC\Share20\Exception\ProviderException: Recipient not in receiving group

Workaround seems to have worked so far, thanks @christianvw for elaborating!

@nilsmelchert
Copy link

This still seems to be a problem. Any progress on fixong this?

@come-nc
Copy link
Contributor

come-nc commented Oct 10, 2022

This still seems to be a problem. Any progress on fixong this?

This needs a refactoring of the way the background job is storing which group-member associations were already announced through the event, so that on login we can add a check that all groups the user is a member of has been announced without having a major performance impact.
So the fix is complicated and the problem not considered blocking, so it will take a bit of time before it’s tackled.

@nilsmelchert

This comment was marked as off-topic.

@come-nc

This comment was marked as off-topic.

@nilsmelchert

This comment was marked as off-topic.

@nilsmelchert

This comment was marked as off-topic.

@nilsmelchert

This comment was marked as off-topic.

@ruedigerkupper
Copy link

Just for my understanding: Do we know, why the paradigm has changed from setting accepted=1 on the group share (as it was before), but letting it at 0 and have a background job do it for all users individually? Why has that changed?

@come-nc
Copy link
Contributor

come-nc commented Oct 13, 2022

Just for my understanding: Do we know, why the paradigm has changed from setting accepted=1 on the group share (as it was before), but letting it at 0 and have a background job do it for all users individually? Why has that changed?

I don’t know, all I can say it the background job is not especially designed for this usecase, it just fires the event signaling that users have been added to the group, which might trigger other things than accepting shares.

@susnux
Copy link
Contributor

susnux commented Dec 23, 2022

By the way if you do not want to use mangle with the database, then there is also an other workaround (other then simply using groupfolders ):

  1. Create a Circle with only that group as Admin
  2. Share the folder with that circle instead

This seems to work fine on our NC25 server.

@szaimen

This comment was marked as resolved.

@szaimen szaimen added needs info 0. Needs triage Pending check for reproducibility or if it fits our roadmap and removed 1. to develop Accepted and waiting to be taken care of labels Jan 23, 2023
@szaimen szaimen closed this as completed Mar 6, 2023
@come-nc come-nc reopened this Mar 9, 2023
@come-nc

This comment was marked as resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap 26-feedback bug feature: ldap feature: sharing
Projects
None yet