Allow sharing with guest users when sharing is limited by user groups#36384
Allow sharing with guest users when sharing is limited by user groups#36384
Conversation
Codecov Report
@@ Coverage Diff @@
## master #36384 +/- ##
============================================
+ Coverage 64.87% 64.87% +<.01%
- Complexity 19791 19797 +6
============================================
Files 1271 1272 +1
Lines 74733 74748 +15
Branches 1309 1309
============================================
+ Hits 48480 48495 +15
Misses 25867 25867
Partials 386 386
Continue to review full report at Codecov.
|
|
We might need to consider to create a |
VicDeo
changed the title
VicDeo
changed the title
|
looks promising. |
karakayasemi
left a comment
karakayasemi
left a comment
There was a problem hiding this comment.
I am not familiar with the Guest app concept, so I am not sure of the concept. In terms of code, rest of the things are good.
karakayasemi
left a comment
karakayasemi
left a comment
There was a problem hiding this comment.
I know, it is not a big deal about performance, but if the user is a guest, no need to make any calculation about group intersection. Because of that, moving this check to one upper level is better in terms of performance.
| /** @var IConfig */ | ||
| private $config; | ||
|
|
||
| public function __construct(IAppManager $appManager = null, IConfig $config = null) { |
There was a problem hiding this comment.
include a comment explaining why the appManager and config can be null.
| */ | ||
| public function isGuestUser($uid) { | ||
| $guestsAppEnabled = $this->appManager->isEnabledForUser('guests'); | ||
| if ($guestsAppEnabled) { |
There was a problem hiding this comment.
should the method return true for the guest user even if the app is disabled? I find it weird that a user is reported as guest, but then, after disabling the app, the user isn't a guest any longer.
| * @throws \Exception | ||
| */ | ||
| protected function userCreateChecks(\OCP\Share\IShare $share) { | ||
| $userTypeHelper = new UserTypeHelper(); |
4 participants
Description
Allow sharing with guest users when
Restrict users to only share with users in their groupsoption is enabledRelated Issue
Motivation and Context
Guests app should not be affected by the ordinary sharing group restriction
How Has This Been Tested?
Restrict users to only share with users in their groupson the settings pagesomeuser@example.org(guest)Expected
User is created, a share is added for this user. No errors
Actual
User is created, but the share is NOT added for this user. Sharing error popup in the web UI
Types of changes
Checklist: