Removing a customgroup removes shares of sharees but not of sharers.

[SergioBertolinSG]

Steps to reproduce

1. Create a custom group 'customgroup'.
2. Add a member to customgroup called 'member1'.
3. Share a folder 'things' with files with group 'customgroup'.
4. in settings remove customgroup 'customgroup'.
5. Go to the files view and check sharing section.
6. Create the customgroup 'customgroup' again.
7. Add a member to customgroup called 'member2'.
8. Log as 'member2', check if you have 'things' shared with you.

Expected behaviour

Folder stops to be shared.

Actual behaviour

Folder is still shared. Sharees doesn't see it shared though.

After step 8, member2 can see the folder 'things' shared with him.

Server configuration

Operating system:
Ubuntu 16.04

Web server:
Apache

Database:
MySQL

PHP version:
7.0

ownCloud version: (see ownCloud admin page)
10.0.3beta1
{"installed":"true","maintenance":"false","needsDbUpgrade":"false","version":"10.0.3.0","versionstring":"10.0.3 beta","edition":"Community","productname":"ownCloud"}

Updated from an older ownCloud or fresh install:
Fresh

The content of config/config.php:

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

Are you using encryption:
No.

Logs

Client configuration

Browser
Chrome

cc @PVince81

[SergioBertolinSG]

Sev2 because if we remove a custom group and we create it again, maybe the old shared folders appear again shared.

[PVince81]

maybe the old shared folders appear again shared.

can you verify if that happens ? if yes, please add it to the steps

[PVince81]

I actually don't think it will happen because the sharing is based on the internal custom groups name which is based on the autoincrement value

[SergioBertolinSG]

can you verify if that happens ? if yes, please add it to the steps

Verified. Changed the steps.

[SergioBertolinSG]

Not happening using regular groups.

[PVince81]

Argh, this is the reason: 3140530

In the past the exposed group id was "customgroup_" + numeric id but now it's "customgroup_" + uri...

Now looking at events, maybe we should trigger group events when deleting groups: "preDelete", "postDelete", etc... This would trigger the logic that automatically deletes shares when a group is deleted.

[PVince81]

If we now start triggering these regular group events, this will likely open a can of worms and we need to be wary of side effects.

[PVince81]

• trial 1: emit the events directly on the group manager from inside the DAV management code
• trial 2: let the custom group backend expose create/delete methods and then use the group manager's create/delete methods for custom group management, which would then internally emit the needed events. (might need to get rid of the symfony dispatcher event that was added there to plug this hole)

[PVince81]

Just tested with trial 2 locally and it seems to work fine.

I guess that eventually we'll want to move other API calls to the group manager to be able to automatically trigger group events: #92

[PVince81]

Raised #91 to trigger more events

[PVince81]

Workaround: when recreating the group, create it with a different name then rename it back to the original name. At group creation, the group URI is generated based on the original name, but doesn't change after renaming.

[PVince81]

Another potential issue though is if someone completely different recreates the group and add themselves, they'd then receive the files originally shared to this group. This could be seen as a security issue.

I still suggest we release the custom groups app version as it is now, as this issue is no regression and existed before. We should then fix this in the release directly after that.

[PVince81]

=> needs to be documented as known issue then, maybe

@pmaier1

[PVince81]

Another potential fix that doesn't involves events would be to append a random hash in the uri to mitigate the problem. However any user who knows the group id of another could still manualy craft a request with the intended URI when recreating the group.

[PVince81]

Bug exists since the first version of customgroups: this commit 3140530 that makes use of uri as core group id exists since the version, I found it in the 0.1 release tarball.

So this bug is not a regression.

[peterprochaska]

@PVince81 @SergioBertolinSG can we fix this asap? This is a public repository, everyone can see this disclosure.