Exclude group backend from showing groups in users page (post-filter approach)

[PVince81]

Description

Some group backends are providing groups only for the share dialog like the customgroups app.
This PR provides a somewhat clumsy way to exclude these from the users page and provisioning API.

Related Issue

owncloud/customgroups#6

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.

Notes

The approach is not good as it also excludes LDAP groups because it relies on the "can this backend manage group memberships" attribute.

We might need a new attribute (but then all old backends must implement it) or a new interface + method to check whether groups can be managed.

@jvillafanez @DeepDiver1975 any advice ?

[mention-bot]

@PVince81, thanks for your PR! By analyzing the history of the files in this pull request, we identified @DeepDiver1975, @bartv2 and @speijnik to be potential reviewers.

[PVince81]

looks like this doesn't exclude from the provisioning API, grrrr

[jvillafanez]

Maybe include some methods such as getVisibleGroup($section, $user) and searchVisibleGroups($section, $user), both defaulting to the current behaviour of showing all the groups?. These calls would be made from the top level, and forwarded to each backend.

Section would refer to well known sections such as "users_page", "sharing_dialog", "provisioning_api", etc. and the user would be the user who wants to fetch the groups (some users shouldn't see some groups)

The different backends would implement this new visiblity method.

I'm not sure if this is a good idea because this might work fine for specific sections, but the backends should know all possible sections, so usage by 3rdparty apps will be difficult.

[jvillafanez]

I don't think we can do this smoothly. If we opt for a breaking change, at least we should make sure the change is extensible enough to not need any additional change for a very long time.

[PVince81]

@jvillafanez thanks for helping to advance the thought process.

This is a nice starting point. My problem currently is that it's not really a "visible group" by itself.
It's more a "manageable by admin in the users page and provisioning API".

Here are a few cases:

• manageable by admin in users page and provisioning API
• can share with these groups (autocomplete, etc)
• can select these groups for options like "enable app for these groups only"
• ... ?

Now since custom groups are basically only to be used for sharing, maybe I could make all backend methods empty so they never return anything. Then add a new method searchGroupsForSharing($pattern, ...) to the custom group backend (defaulting to default search), which is only used by the various sharing code paths. This is similar to your idea @jvillafanez but the other way around.

[PVince81]

I'm going to try out the above proposal, there is a bit of chance that it might work.

I only found one code path for the share autocomplete. However if we want to be strict about validation, we'll also need to tweak the share manager to not allow creating direct shares with groups not returned by searchGroupsForSharing().

[jvillafanez]

If we have a base class OC\Group, we could have a subclass OC\VisibilityGroup which includes a isVisible(...) method. Core or any other app could check if the group returned by the top level group manager is a normal OC\Group or if it's a OC\VisiblityGroup and decide if it will be shown or not.

[PVince81]

@jvillafanez while this sounds like it makes sense, the Group instance is created by the IGroupManager implementation, not individual group backends. We'd additionally need a way for the group backend to tell in its response that a group is a VisibilityGroup and not a Group. Also missing is that the group backend cannot then decide the real implementation of VisibilityGroup as it suits. 😦

[PVince81]

I'm starting to get a feeling that we're abusing the "group backend" thing for the sake of custom groups.

The other approach is more complicated unfortunately: it requires to implement:

• a new share provider
• a new share type "share with custom group"
• a new mount provider to retrieve and resolve all shares to be mounted for that user (and that includes deduplication, etc... a lot of logic we already have in current mount provider)

[PVince81]

@jvillafanez PR with the "getUserGroupsForSearch" approach: owncloud/customgroups#25

I'm not happy with that one either, but at least it works and properly hides the groups from management APIs.

[PVince81]

Obsoleted by the better #27287

[lock]

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.