Hides nodes from listing that the user has no access to#22731
Hides nodes from listing that the user has no access to#22731DeepDiver1975 merged 1 commit intomasterfrom
Conversation
|
By analyzing the blame information on this pull request, we identified @DeepDiver1975, @blizzz and @PVince81 to be potential reviewers |
LukasReschke
changed the title
LukasReschke
force-pushed
the
hide-nodes-from-listening
branch
from
6a82ca7 to
a264af0
Compare
|
Ok. I'll adjust this to completely cover #22578. Should be easier than thought… |
LukasReschke
force-pushed
the
hide-nodes-from-listening
branch
3 times, most recently
from
ae3e0f3 to
b10a703
Compare
|
@DeepDiver1975 Please take a look. THX. |
|
Nice. Tested a few webdav calls and 404 is returned properly now. (So Basically did the intergration tests by hand :P). 👍 And yes we need to look at the intergration tests ;) |
LukasReschke
force-pushed
the
hide-nodes-from-listening
branch
3 times, most recently
from
1742595 to
4f4086c
Compare
|
Yay. The integration tests are failing due to the fact that https://github.com/php/php-src/blob/97294aca7e066443291cc2d77f8674ac23eabb32/sapi/cli/php_http_parser.c#L78-L105 does not support MKCALENDAR. Adjusting this PR and filing one to PHP then…… |
|
Well. There is already one at php/php-src#1417. Anyways, PR has been adjusted. |
LukasReschke
force-pushed
the
hide-nodes-from-listening
branch
from
4f4086c to
d04edfa
Compare
|
Tested using cadaver and LDAP users, works 👍 |
| $node = $this->server->tree->getNodeForPath($uri); | ||
|
|
||
| switch(get_class($node)) { | ||
| case 'OCA\DAV\CardDAV\AddressBook': |
There was a problem hiding this comment.
SabreDAV uses "Node" there as error message which is the default. Apparently only for AddressBooks it uses a custom error message.
We have integration tests in place below to ensure that we notice the case when SabreDAV changes the error message.
Hides nodes from listing that the user has no access to
|
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. |
5 participants
Fixes #22578, most of it are integration tests. The integration tests are not nice but do work ™️
… we need at some point clean them up a bit, the existing structure with the "use" trait is rather strange IMHO …
@DeepDiver1975 THX