do not urlencode group id

[butonic]

fixes #19

Text added by phil-davis:
Also see core issue owncloud/core#31279 - groups with / or % in the name do not list the members

These problems are here in the user_management app, and also in core stable10 (which still has this user management code bundled in core)

[CLAassistant]

All committers have signed the CLA.

[butonic]

backport for core stable10 in owncloud/core#31109

[DeepDiver1975]

please sign cla - THX

[phil-davis]

works for me - see comments owncloud/core#31109 (comment)

[DeepDiver1975]

acceptance tests are failing ...

[PVince81]

did we not recently switch to encoding it, and now we revert that again ? somehow can't find the PR

[PVince81]

are we at least encoding in another location before sending to server ?

[PVince81]

never mind, I think this proof is enough owncloud/core#31109 (comment)

[phil-davis]

I am working to fix whatever acceptance CI needs refactoring for recent core changes:
#22

When that is good again, then this can be rebased and we can see if there are actually any real issues/regressions.

[phil-davis]

PR #22 has fixed the acceptance tests here on top of recent core changes. Rebased this PR.

[codecov-io]

Codecov Report

Merging #20 into master will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master      #20   +/-   ##
=========================================
  Coverage     76.86%   76.86%           
  Complexity      186      186           
=========================================
  Files            19       19           
  Lines           765      765           
=========================================
  Hits            588      588           
  Misses          177      177

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 914f285...7a7f6f6. Read the comment docs.

[phil-davis]

Before this PR, groups with / or % in the group name could be deleted on the webUI.
After this PR they can no longer be deleted.
This is why the acceptance tests fail.

So there is more detail think about.

[phil-davis]

That last commit should fix it - encodeURIComponent() is only needed when we are sending off a delete. In the other cases where we are just displaying groups and their members we use the ordinary "raw" ASCII group name.

And I added a commit to test deleting a group with a space in the name on the webUI, since that is an obvious deletion case that is not currently there.

[butonic]

User listing for groups with / and % is also broken.

@phil-davis hm if only delete is affected then the problem seems to be caused by the DeleteHandler ... so what does that do: https://github.com/owncloud/user_management/blob/7a69da7e49a8d76e4d9310dcb68d8e4872938a18/js/deleteHandler.js#L194

It uses OC.generateUrl, which already does encodeURIComponent ... maybe the serverside? Aha:
https://github.com/owncloud/user_management/blob/cc021358a789be720732696c30c48ca2b4230c04/lib/Controller/GroupsController.php#L129-L131

double urldecode ... the implicit one by the framework and this one. I removed it bte it seems I cannot delete groups with / ... the request fails with a 404 which means something in the resource route matching is fishy.

[butonic]

[Fri Apr 20 11:17:10.184979 2018] [core:info] [pid 24476] [client 127.0.0.1:47726] AH00026: found %2f (encoded '/') in URI (decoded='/index.php/settings/users/groups/foo/bar'), returning 404

WTF

[phil-davis]

I just tried:

• created groups called a/slash and a%percent
• put some users in them
• listed them on the webUI
• deleted them on the webUI

with current core master and this branch of user_management

[butonic]

and then the Router throws a ResourceNotFoundException because the Symphony URLMapper does a rawurldecode before matching ...

will update this PR and open core issue

[butonic]

also needs AllowEncodedSlashes On to get around #20 (comment)

[butonic]

@phil-davis yes this branch currently works, but there is a more fundamental problem

[butonic]

superseded by owncloud/core#31224

[phil-davis]

This is also needed so that the front-end sends the correct encoding (or not) for the various requests.
Matching core stable10 PR owncloud/core#31109

Re-opened and re-based.

[PVince81]

Code looks good 👍

[PVince81]

@ownclouders rebase

[phil-davis]

I rebased locally just now and force pushed. Let's see what CI thinks.
@PVince81 @DeepDiver1975 core master owncloud/core#31224 was just merged. So this should be "a good thing" to go with it.