Allow blocking of group sharing #23398
Conversation
| @@ -264,6 +264,10 @@ public function createShare() { | |||
| $share->setSharedWith($shareWith); | |||
| $share->setPermissions($permissions); | |||
| } else if ($shareType === \OCP\Share::SHARE_TYPE_GROUP) { | |||
| if (!$this->shareManager->allowGroupSharing()) { | |||
| return new \OC_OCS_Result(null, 404, 'group sharing is disabled by the administrator'); | |||
There was a problem hiding this comment.
I'd remove the administrator part, that's obvious for each "disabled"setting
There was a problem hiding this comment.
It is just there out of consistency... (it is basically the same message as for disabled public upload etc)
|
+1 |
|
Integration tests missing :shocked: :P |
|
Yeah... That is what you get with all this last minute stuff... |
|
@nickvergessen but good idea... will tackle that on monday |
| * @return bool | ||
| * @since 9.0.1 | ||
| */ | ||
| public function allowGroupSharing(); |
There was a problem hiding this comment.
Let's hope no one implemented their own ShareManager in 9.0.0 😉
I guess this addition should be fine.
There was a problem hiding this comment.
They could but then they would have to patch core anyways... we only support custom shareProviders ;)
There was a problem hiding this comment.
A less-bc-breaking approach would have been to have a getOptions() method which returns an array. This way we can expose the options in the future without breaking the interface.
There was a problem hiding this comment.
I must say I like that...
There was a problem hiding this comment.
But has the disadvantage of missing type hints. Could do it like with storages and add seperate interfaces which you can type hint against?
There was a problem hiding this comment.
Ok - we already have a ton of methods like this ...... 👍
|
Code looks good so far. Will have a quick test later after integration tests popped up. 😄 |
rullzer
force-pushed
the
block_group_sharing
branch
from
eb42b14
to
15bf1ef
Compare
|
👍 |
|
Tested, works. Small things:
Maybe there are other locations that need adjusting ? |
|
and something fishy with the integration tests: |
fails ❌ |
|
Aah that is appareantly because the sharing stuff is a trait and not a class... so the afterScenario stuff is not called... :S |
|
My behat fu is not good enough here. I have a feeling we need to define a proper shareContext. And not just have it as a trait. Because for some reason then everything goes wrong... @nickvergessen lets discuss tomorrow |
rullzer
force-pushed
the
block_group_sharing
branch
2 times, most recently
from
5839912
to
213248f
Compare
|
Ok we need to do some more refactoring in the tests to get them to work. So discussed with @nickvergessen and no share intergration tests for now... to make sure we can still backport this. |
|
My 👍 stands, @PVince81 ? |
|
👍 |
|
@karlitschek backport to 9.0.1 ? |
Required for https://github.com/owncloud/enterprise/issues/1079 |
|
please backport |
|
Stable 9 in #23473 |
* Only for sharees right now * Sharing intergration tests fail due to the test setup we have right now
|
@rullzer conflicting - please rebase - THX |
rullzer
force-pushed
the
block_group_sharing
branch
from
213248f
to
cf3e740
Compare
|
Tested and works 👍 |
|
@DeepDiver1975 done... the mysql failure is the known one.. |
Allow blocking of group sharing
|
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. |
Fixes https://github.com/owncloud/enterprise/issues/1079
FYI: @cmonteroluque @MTRichards @cdamken
This introduces an extra config variable to core.
If disabled the share API will block group shares (and the manager as well to avoid apps doing group shares).
It also disables the sharee API to disable group shares
Tests are updated and extended.
Please review: @PVince81 @nickvergessen @MorrisJobke