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

Group name vs display name #7918

Closed
RobinMcCorkell opened this issue Mar 27, 2014 · 15 comments
Closed

Group name vs display name #7918

RobinMcCorkell opened this issue Mar 27, 2014 · 15 comments

Comments

@RobinMcCorkell
Copy link
Member

The database users and groups storage represents a user with a 'username' and a 'full name', where the username is deemed unique and used as a UID. The same is not true of groups - there is only a 'group name', which must be unique but should be the pretty name.

This causes problems with LDAP, where groups are defined using a unique attribute and (sometimes) a display name. Setting the advanced options in user_ldap to use displayName as the group name makes ownCloud start using that display name in equivalent places to where a user's UID is used, such as mount configurations. The current workaround is to use a unique attribute for the group display name, but this isn't desirable as unique =/= pretty.

It would be useful for groups to be treated similarly to users, with a unique 'group name' and a 'display name', which are independent of each other.

@blizzz
Copy link
Contributor

blizzz commented Mar 28, 2014

Yes, it is reasonable to extend the display name to groups. Since the handling for users is there and groups are likely a bit easier to deal with in this regard, it should not be too difficult to get it done.

@RobinMcCorkell
Copy link
Member Author

Perhaps a set of functions, similar to the ones available for users, such as group->getDisplayName() and the like. Then the database provider for groups can just provide the same display name as the group name, while LDAP (or other user providers) can extend that to return the actual group name.

@PVince81
Copy link
Contributor

@DeepDiver1975 @butonic we will need a display name for the custom groups feature to be able to support renaming. The group id would be a generated id but the display name entered by the users.

Setting to 9.2

@PVince81
Copy link
Contributor

Currently the main problem is that the group backend interface \OCP\GroupInterface only works with group ids, some methods just returns an array of group ids, etc. The interface needs to be extended, maybe as a brand new interface \OCP\IGroupsBackend which itself works with \OCP\IGroup objects instead of just group ids.

Then we extend \OCP\IGroup to also have a display name attribute + accessors. Might need a new interface as well OCP\IGroup2 or something to avoid breaking old ones.

Or for all have brand new interfaces and write an adapter to keep compatibility with older group backends. The same should be done at some point for the user backend as well, but not as part of this one task.

@PVince81
Copy link
Contributor

Oh no... I looked into the code and it looks worse than I thought.

Actually the private Group class doesn't represent a single group but all groups that might exist across multiple backends. So if two backends have a group "group1" it would store them both as a single Group instance. Then if you query its methods, it will iterate over the backends to fetch the info.

So it is not suitable to store the display name as part of this instance.

But the part that worries me most is about already supporting duplicate group ids from different backends. While this set of classes seem to support it it will likely explode on other layers...

So far, need to find a new home for the display name attribute then. I hope we won't need to reinvent the whole user / group manager thing...

@PVince81
Copy link
Contributor

Interestingly the user backend only supports single ids.

Now I'd be tempted to say that starting with 9.2 we won't support duplicate group names any more. Or if we introduce display names anyway, how about namespacing them by backend id ?

For example LDAP groups would use the $gid "user_ldap_somegroup1" for the LDAP group "somegroup1".
Local DB groups would use the $gid "local_somegroup1".
Then in a migration step we make sure that the old name is used as the display name so that all UI would still show the original name. Sounds like a lot of work which might break stuff...

@PVince81
Copy link
Contributor

Ok forget about namespacing for now, it's too complicated.

Let's say if there are setups out there that do have multiple group names, only one can win, the first backend would win. So everywhere the group id was specified like in share_with column would anyway be resolved to the first one. So what we can do is enforce that only the first group backend wins.
Maybe some warning needs to be displayed for the admin in case of duplicate groups...

@PVince81
Copy link
Contributor

Just did some testing: create a local group "Box10" then setup LDAP where there's also a group "Box10".
The result is that the LDAP mapper maps it to a new group "Box10_2" locally... clever.

But it still doesn't prevent a non-clever backend to just return the same group name.

Ok, and if I try to add a local user into a LDAP group it just does nothing and disappears on refresh.

At first I thought this "feature" of having multiple backends providing the same group id was to somehow allow adding people of one group backend into the group of another backend. But no.

Good.

@PVince81
Copy link
Contributor

Uh oh... while I can't add a local user to a LDAP group, I can add a LDAP user to a local group, the user appears in the oc_group_user. Might be fine though. I think it's not related to the multi-backend group then.

@PVince81
Copy link
Contributor

Well if we HAD a group backend somewhere out there that would simply return group ids without preprocessing conflicts: it would be possible to add local users to any of them because for OC it is virtually only a single group. So any user added to the group would appear in oc_group_user with the group id.

So maybe that logic of merging the groups into a single virtual group does (or did in the past) make sense.

But the sad part is that the virtual group doesn't have any entry in the database. So maybe we also need a group table that contains the groups from all backends ? (mentioned in the user account table ticket here #23558 (comment)).

Or should there be a mapping table that creates a local alias for any remote group, and takes care that the alias is unique ?

@PVince81
Copy link
Contributor

Adding a display name and potentially further group info means that we also need to "merge" this info somehow into the virtual group, which isn't really possible in case they are different.

Maybe in the case of the display name we only take the one from the first backend then in case of conflict ?

@PVince81
Copy link
Contributor

WIP PR here #26750

@PVince81
Copy link
Contributor

PVince81 commented Nov 30, 2016

  • backend interface support: Adding group display name support in group backends #26750
  • add to local backend
    • new DB column in oc_groups for display names
  • add display name support in various places
    • users page
      • display name in dropdowns (+ group id as tooltip)
      • rename icon in left panel if backend supports display name / renaming
      • somehow let the user specify both group id + display name when creating new groups
    • various dropdowns (exclude groups from sharing, exclude groups from apps)
    • sharing autocomplete / sharee API: Adding group display name support in group backends #26750
  • import from LDAP if available ?

@PVince81
Copy link
Contributor

Display name support was added in backend, so this is there.

For the remaining tasks let's use the other ticket #12600

@lock
Copy link

lock bot commented Aug 3, 2019

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.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants