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 names containing vertical bar cannot be used in "exclude groups from sharing" #31836

Closed
phil-davis opened this issue Jun 20, 2018 · 9 comments

Comments

@phil-davis
Copy link
Contributor

phil-davis commented Jun 20, 2018

Steps to reproduce

  1. create a group named vertical|bar
  2. put some users in it (just for fun)
  3. admin, settings, sharing, enable "Exclude groups from sharing"
  4. In the box, type the group vertical|bar and then exxit the box (to save)
  5. refresh the page

Expected behaviour

Group vertical|bar should be shown in the list of excluded groups.

Also note: on the user management page, the users in group vertical|bar do not show.

Actual behaviour

Groups vertical and bar are shown as separate group names.

Server configuration

Current core stable10

Note: because of issue #31739 the whole "exclude groups from sharing" UI is broken in core master
So you currently have to use stable10 to see the issue reported here.

@phil-davis
Copy link
Contributor Author

How many of these "special character in group name" problems are there?
Would anyone ever actually use a vertical bar in a group name?
Should we ban the vertical bar as a character in a group name?
But maybe such group names are valid in LDAP, and so they could come from an LDAP server anyway.

@ownclouders
Copy link
Contributor

GitMate.io thinks possibly related issues are #10307 (Exclude Groups from Sharing), #20473 (If a user belongs to a group that has not been set in the excluded groups from sharing, he can share files), #25320 ('Exclude groups from sharing' doesn't work if user belongs to more groups), #17346 (Exclude group from sharing after a share was created), and #10983 (Exclude with groups cannot work with groups containing commas).

@phil-davis
Copy link
Contributor Author

phil-davis commented Jun 20, 2018

@phil-davis
Copy link
Contributor Author

PR #10968 introduced the vertical bar implode code, which is better than the previous comma separator.

The answer to my questions #31836 (comment) will determine the way forward.

@PVince81
Copy link
Contributor

From what I heard, all kinds of weird characters can come from various flavors of LDAP servers.

We still do have the option to say "yes, maybe, but OC doesn't support that".

Now with the latest discussions related to special chars in groups, let's include @tomneedham @butonic in the discussion...

@PVince81 PVince81 added this to the backlog milestone Jun 21, 2018
@PVince81
Copy link
Contributor

the question could also be rephrased: is there any safe character that is guaranteed to never appear in a LDAP group name ? such char could then be used as separator

@VicDeo
Copy link
Member

VicDeo commented Oct 1, 2019

use a regex to split the value
For backend it's trivial. For frontend https://stackoverflow.com/a/14334054

groups with \| are rarer than with an ordinary pipe

@phil-davis
Copy link
Contributor Author

And when there is a group with \ then escape that. So 4 groups group1 back\slash vertical|bar and oh\|heck becomes:
group1|back\\slash||vertical\bar|oh\\\|heck
or some similar escaping scheme.

@stale
Copy link

stale bot commented Sep 21, 2021

This issue has been automatically closed.

@stale stale bot closed this as completed Sep 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants