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

Correctly remove usergroup shares on removing group members #22015

Conversation

nickvergessen
Copy link
Member

Steps

  1. As admin create a group and assign userA to the group
  2. Share a file to the group
  3. As userA edit the file
  4. As admin remove userA from the group
  5. Check share table (there is still a usergroup share entry for userA before this PR)
  6. As admin modify the file
  7. As userA check your activity (there is a new entry before this PR)

There seems to be inconsistency for LDAP and other group backends. The listened old hook 'OC_User', 'post_removeFromGroup' is not emitted in the normal user/group backend since 16 at least?

@faily-bot
Copy link

faily-bot bot commented Jul 27, 2020

🤖 beep boop beep 🤖

Here are the logs for the failed build:

Status of 31122: failure

integration-auth

  • build/integration/features/auth.feature:70
Show full log
  Scenario: using OCS anonymously                                                    # /drone/src/build/integration/features/auth.feature:70
[Mon Jul 27 11:18:22 2020] 127.0.0.1:41962 [404]: /ocs/v2.php/cloud/users/user0
[Mon Jul 27 11:18:22 2020] 127.0.0.1:41986 [200]: /ocs/v1.php/cloud/users
[Mon Jul 27 11:18:23 2020] 127.0.0.1:42014 [200]: /ocs/v1.php/cloud/users/user0
[Mon Jul 27 11:18:23 2020] 127.0.0.1:42032 [200]: /ocs/v2.php/cloud/users/user0
[Mon Jul 27 11:18:23 2020] 127.0.0.1:42044 [200]: /login
[Mon Jul 27 11:18:23 2020] 127.0.0.1:42046 [303]: /login
[Mon Jul 27 11:18:23 2020] 127.0.0.1:42056 [200]: /index.php/apps/files/
[Mon Jul 27 11:18:23 2020] 127.0.0.1:42060 [200]: /index.php/settings/personal/authtokens
[Mon Jul 27 11:18:23 2020] 127.0.0.1:42064 [200]: /index.php/settings/personal/authtokens/56
[Mon Jul 27 11:18:23 2020] 127.0.0.1:42066 [303]: /login
[Mon Jul 27 11:18:23 2020] 127.0.0.1:42068 [200]: /index.php/apps/files/
[Mon Jul 27 11:18:29 2020] 127.0.0.1:42072 [303]: /login
[Mon Jul 27 11:18:29 2020] 127.0.0.1:42164 [200]: /index.php/apps/files/
[Mon Jul 27 11:18:29 2020] 127.0.0.1:42170 [200]: /index.php/settings/personal/authtokens
[Mon Jul 27 11:18:29 2020] TypeError: Argument 11 passed to OCA\Files_Sharing\External\Manager::__construct() must be of the type string, null given, called in /drone/src/apps/files_sharing/lib/AppInfo/Application.php on line 98 at /drone/src/apps/files_sharing/lib/External/Manager.php#91
[Mon Jul 27 11:18:29 2020] 127.0.0.1:42174 [200]: /ocs/v1.php/apps/files_sharing/api/v1/remote_shares
    When requesting "/ocs/v1.php/apps/files_sharing/api/v1/remote_shares" with "GET" # FeatureContext::requestingWith()
    Then the OCS status code should be "997"                                         # FeatureContext::theOCSStatusCodeShouldBe()
      Notice: Trying to get property 'meta' of non-object in /drone/src/build/integration/features/bootstrap/BasicStructure.php line 153
[Mon Jul 27 11:18:29 2020] 127.0.0.1:42178 [200]: /ocs/v1.php/cloud/users/user0
[Mon Jul 27 11:18:29 2020] 127.0.0.1:42218 [404]: /ocs/v2.php/cloud/users/user0
[Mon Jul 27 11:18:29 2020] Login failed: 'user0' (Remote IP: '127.0.0.1')
[Mon Jul 27 11:18:29 2020] 127.0.0.1:42234 [401]: /remote.php/webdav/myFileToComment.txt
[Mon Jul 27 11:18:30 2020] 127.0.0.1:42242 [207]: /remote.php/dav/systemtags/
[Mon Jul 27 11:18:30 2020] Login failed: 'user0' (Remote IP: '127.0.0.1')
[Mon Jul 27 11:18:30 2020] 127.0.0.1:42264 [401]: /remote.php/webdav/myFileToTag.txt
[Mon Jul 27 11:18:30 2020] 127.0.0.1:42266 [404]: /remote.php/dav/addressbooks/users/admin/MyAddressbook
[Mon Jul 27 11:18:30 2020] 127.0.0.1:42294 [404]: /remote.php/dav/calendars/admin/MyCalendar

Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and verified. Backport down to stable18 should work (the UserRemovedEvent event was added then)

Signed-off-by: Joas Schilling <coding@schilljs.com>
@MorrisJobke MorrisJobke force-pushed the bugfix/noid/correctly-remove-user-shares-on-removing-group-members branch from f24ee81 to 5993bd4 Compare July 30, 2020 07:47
@MorrisJobke
Copy link
Member

Rebased and squashed

@MorrisJobke MorrisJobke merged commit 7883665 into master Jul 30, 2020
@MorrisJobke MorrisJobke deleted the bugfix/noid/correctly-remove-user-shares-on-removing-group-members branch July 30, 2020 07:47
@MorrisJobke
Copy link
Member

/backport to stable19

@MorrisJobke
Copy link
Member

/backport to stable18

@MorrisJobke
Copy link
Member

(the UserRemovedEvent event was added then)

#18350

@MorrisJobke
Copy link
Member

There seems to be inconsistency for LDAP and other group backends. The listened old hook 'OC_User', 'post_removeFromGroup' is not emitted in the normal user/group backend since 16 at least?

In the DB backend it is only triggered on user deletion:

\OC_Hook::emit("OC_User", "post_removeFromGroup", ["uid" => $this->uid, "gid" => $groupId]);

@MorrisJobke
Copy link
Member

The user removal from a group is a different event:

$this->dispatcher->dispatch(IGroup::class . '::postRemoveUser', new GenericEvent($this, [
'user' => $user,
]));
if ($this->emitter) {
$this->emitter->emit('\OC\Group', 'postRemoveUser', [$this, $user]);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants