dont allow user to override admin sharing settings when its disabled #34764

karakayasemi · 2019-03-14T19:22:20Z

Description

Implementation of https://github.com/owncloud/enterprise/issues/3128#issuecomment-471185144

Hide sharing personal panel options from users if they are disabled by admin as a security concern.
If they are enabled, allow users to make their own choice.
Fix small UI issues on federatedfilesharing admin panel Automatically accept remote shares from trusted servers always enabled in sharing admin panel #34763
Fix inconcistency between user and admin selection user has to disable and enable share autoaccept setting to override disabled admin setting #34708

If all the options are disabled by admin, instead of showing empty page, I added Nothing to configure sentence for each section.

To-do:

Fix acceptance tests

Related Issue

Fixes https://github.com/owncloud/enterprise/issues/3128
Closes #34763
Closes #34708

Motivation and Context

How Has This Been Tested?

Disable auto_accept options from admin panel
User should see Nothing to configure text in sharing personal panel
Enable auto_accept options from admin panel
User should allowed to make their own choice

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Database schema changes (next release will require increase of minor version instead of patch)
Breaking change (fix or feature that would cause existing functionality to change)
Technical debt
Tests only (no source changes)

Checklist:

Code changes
Unit tests added
Acceptance tests added
Documentation ticket raised:

Open tasks:

Backport (if applicable set "backport-request" label and remove when the backport was done)

karakayasemi · 2019-03-14T19:23:00Z

apps/federatedfilesharing/lib/Panels/AdminPanel.php

@@ -57,7 +57,7 @@ public function getPanel() {
 		$tmpl->assign('incomingServer2serverShareEnabled', $this->shareProvider->isIncomingServer2serverShareEnabled());
 		$tmpl->assign(
 			'autoAcceptTrusted',
-			$this->config->getAppValue('federatedfilesharing', 'auto_accept_trusted', 'no')
+			$this->config->getAppValue('federatedfilesharing', 'auto_accept_trusted', 'no') === 'yes'


fix for #34763

apps/federatedfilesharing/lib/Panels/SharingPersonalPanel.php

apps/federatedfilesharing/templates/settings-personal-sharing.php

jvillafanez · 2019-03-15T08:35:44Z

for the template, how about having 2 different templates, one for the "nothing to configure" and another one for the normal content? I'm not sure it this approach can help with the unittests and simplify the code.

In addition, you're following the same approach for the federated sharing and "normal" sharing, is this what we want? (just checking)

karakayasemi · 2019-03-15T08:53:00Z

for the template, how about having 2 different templates, one for the "nothing to configure" and another one for the normal content? I'm not sure it this approach can help with the unittests and simplify the code.

I think it makes sense. Using two different template can reduce template and panel complexity.

In addition, you're following the same approach for the federated sharing and "normal" sharing, is this what we want? (just checking)

According to the agreement on this comment https://github.com/owncloud/enterprise/issues/3128#issuecomment-471185144, it is desired same functionality for both federated and local sharing.

codecov · 2019-03-15T15:01:40Z

Codecov Report

Merging #34764 into master will increase coverage by <.01%.
The diff coverage is 94.59%.

@@             Coverage Diff              @@
##             master   #34764      +/-   ##
============================================
+ Coverage      65.3%   65.31%   +<.01%     
- Complexity    18479    18483       +4     
============================================
  Files          1209     1209              
  Lines         69971    69996      +25     
  Branches       1280     1280              
============================================
+ Hits          45695    45715      +20     
- Misses        23904    23909       +5     
  Partials        372      372

Flag	Coverage Δ	Complexity Δ
#javascript	`53.04% <ø> (ø)`	`0 <ø> (ø)`	⬇️
#phpunit	`66.71% <94.59%> (ø)`	`18483 <0> (+4)`	⬆️

Impacted Files	Coverage Δ	Complexity Δ
apps/files_sharing/templates/settings-personal.php	`25% <0%> (-41.67%)`	`0 <0> (ø)`
...ilesharing/templates/settings-personal-sharing.php	`25% <0%> (-41.67%)`	`0 <0> (ø)`
...es_sharing/lib/Controller/Share20OcsController.php	`85.39% <100%> (+0.05%)`	`194 <0> (+1)`	⬆️
...tedfilesharing/lib/Panels/SharingPersonalPanel.php	`100% <100%> (ø)`	`5 <0> (+1)`	⬆️
...ederatedfilesharing/lib/FederatedShareProvider.php	`61.13% <100%> (+0.17%)`	`85 <0> (+1)`	⬆️
...pps/federatedfilesharing/lib/Panels/AdminPanel.php	`100% <100%> (ø)`	`4 <0> (ø)`	⬇️
...iles_sharing/lib/Panels/Personal/PersonalPanel.php	`100% <100%> (ø)`	`5 <0> (+1)`	⬆️
...eratedfilesharing/lib/Controller/OcmController.php	`66.06% <0%> (-0.21%)`	`30% <0%> (ø)`
lib/private/Server.php	`86.56% <0%> (-0.12%)`	`253% <0%> (ø)`

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 4ac13e2...8db3515. Read the comment docs.

codecov · 2019-03-15T15:01:47Z

Codecov Report

Merging #34764 into master will increase coverage by <.01%.
The diff coverage is 81.39%.

@@             Coverage Diff              @@
##             master   #34764      +/-   ##
============================================
+ Coverage     65.31%   65.31%   +<.01%     
- Complexity    18481    18487       +6     
============================================
  Files          1209     1209              
  Lines         69983    70009      +26     
  Branches       1280     1280              
============================================
+ Hits          45706    45726      +20     
- Misses        23905    23911       +6     
  Partials        372      372

Flag	Coverage Δ	Complexity Δ
#javascript	`53.04% <ø> (ø)`	`0 <ø> (ø)`	⬇️
#phpunit	`66.72% <81.39%> (ø)`	`18487 <0> (+6)`	⬆️

Impacted Files	Coverage Δ	Complexity Δ
apps/files_sharing/templates/settings-personal.php	`20% <0%> (-46.67%)`	`0 <0> (ø)`
...ilesharing/templates/settings-personal-sharing.php	`20% <0%> (-46.67%)`	`0 <0> (ø)`
...es_sharing/lib/Controller/Share20OcsController.php	`85.39% <100%> (+0.05%)`	`194 <0> (+1)`	⬆️
...iles_sharing/lib/Panels/Personal/PersonalPanel.php	`100% <100%> (ø)`	`6 <0> (+2)`	⬆️
...pps/federatedfilesharing/lib/Panels/AdminPanel.php	`100% <100%> (ø)`	`4 <0> (ø)`	⬇️
...ederatedfilesharing/lib/FederatedShareProvider.php	`61.13% <100%> (+0.17%)`	`85 <0> (+1)`	⬆️
...tedfilesharing/lib/Panels/SharingPersonalPanel.php	`100% <100%> (ø)`	`6 <0> (+2)`	⬆️

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 898c68f...ce02210. Read the comment docs.

PVince81 · 2019-03-15T16:47:51Z

some sharing tests failed, @karakayasemi please have a look.

if you need assistance with adjusting or extending UI tests, feel free to ask @individual-it or @phil-davis

karakayasemi · 2019-03-16T15:32:58Z

@individual-it I tried to adapt acceptance tests to the new behavior, removed some of the issue tags.
I read the documentation about acceptance testing, it is pretty explanatory by the way. But, I have no prior experience in it. Please review, acceptance tests part. Thanks.

tests/acceptance/features/bootstrap/WebUIPersonalSharingSettingsContext.php

individual-it · 2019-03-18T08:50:35Z

the tests look very good to me, just don't like the long lines

very good, achievement unlocked 🎉

individual-it · 2019-03-18T10:00:12Z

👍 for the acceptance tests part

karakayasemi · 2019-03-18T11:47:46Z

@jvillafanez I adjusted the code to your last review. Templates and panel simplified with creating two templates. Please review, one more time. Thanks.

apps/federatedfilesharing/templates/settings-personal-sharing.php

apps/federatedfilesharing/tests/Panels/SharingPersonalPanelTest.php

karakayasemi · 2019-03-19T13:52:40Z

@jvillafanez the code adjusted to your last suggestions, please review one more time. Thanks.

karakayasemi added 3 - To Review backport-request labels Mar 14, 2019

karakayasemi self-assigned this Mar 14, 2019

karakayasemi requested review from PVince81, VicDeo and jvillafanez March 14, 2019 19:22

karakayasemi commented Mar 14, 2019

View reviewed changes

jvillafanez reviewed Mar 15, 2019

View reviewed changes

apps/federatedfilesharing/lib/Panels/SharingPersonalPanel.php Outdated Show resolved Hide resolved

apps/federatedfilesharing/templates/settings-personal-sharing.php Outdated Show resolved Hide resolved

karakayasemi force-pushed the fix-3128 branch from 969ea18 to 8db3515 Compare March 15, 2019 15:01

karakayasemi force-pushed the fix-3128 branch 3 times, most recently from 879ff74 to 322ca36 Compare March 15, 2019 15:40

karakayasemi force-pushed the fix-3128 branch from 322ca36 to d5fbb45 Compare March 16, 2019 15:24

karakayasemi requested a review from individual-it March 16, 2019 15:26

karakayasemi requested a review from jvillafanez March 16, 2019 16:46

individual-it reviewed Mar 18, 2019

View reviewed changes

tests/acceptance/features/bootstrap/WebUIPersonalSharingSettingsContext.php Outdated Show resolved Hide resolved

karakayasemi force-pushed the fix-3128 branch from d5fbb45 to 03165d1 Compare March 18, 2019 09:43

karakayasemi requested a review from individual-it March 18, 2019 09:47

jvillafanez reviewed Mar 18, 2019

View reviewed changes

apps/federatedfilesharing/templates/settings-personal-sharing.php Outdated Show resolved Hide resolved

apps/federatedfilesharing/tests/Panels/SharingPersonalPanelTest.php Outdated Show resolved Hide resolved

dont allow user to override admin sharing settings when its disabled

ce02210

karakayasemi force-pushed the fix-3128 branch from 03165d1 to ce02210 Compare March 18, 2019 13:17

karakayasemi requested a review from jvillafanez March 18, 2019 13:24

jvillafanez approved these changes Mar 20, 2019

View reviewed changes

karakayasemi merged commit 86b49ab into master Mar 20, 2019

delete-merged-branch bot deleted the fix-3128 branch March 20, 2019 08:45

karakayasemi mentioned this pull request Mar 20, 2019

[stable10] dont allow user to override admin sharing settings when its disabled #34842

Merged

11 tasks

karakayasemi removed the backport-request label Mar 20, 2019

lock bot locked as resolved and limited conversation to collaborators Mar 21, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dont allow user to override admin sharing settings when its disabled #34764

dont allow user to override admin sharing settings when its disabled #34764

karakayasemi commented Mar 14, 2019 •

edited

Loading

karakayasemi Mar 14, 2019

jvillafanez commented Mar 15, 2019

karakayasemi commented Mar 15, 2019 •

edited

Loading

codecov bot commented Mar 15, 2019

codecov bot commented Mar 15, 2019 •

edited

Loading

PVince81 commented Mar 15, 2019

karakayasemi commented Mar 16, 2019 •

edited

Loading

individual-it commented Mar 18, 2019

individual-it commented Mar 18, 2019

karakayasemi commented Mar 18, 2019

karakayasemi commented Mar 19, 2019

dont allow user to override admin sharing settings when its disabled #34764

dont allow user to override admin sharing settings when its disabled #34764

Conversation

karakayasemi commented Mar 14, 2019 • edited Loading

Description

To-do:

Related Issue

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

Open tasks:

karakayasemi Mar 14, 2019

Choose a reason for hiding this comment

jvillafanez commented Mar 15, 2019

karakayasemi commented Mar 15, 2019 • edited Loading

codecov bot commented Mar 15, 2019

Codecov Report

codecov bot commented Mar 15, 2019 • edited Loading

Codecov Report

PVince81 commented Mar 15, 2019

karakayasemi commented Mar 16, 2019 • edited Loading

individual-it commented Mar 18, 2019

individual-it commented Mar 18, 2019

karakayasemi commented Mar 18, 2019

karakayasemi commented Mar 19, 2019

karakayasemi commented Mar 14, 2019 •

edited

Loading

karakayasemi commented Mar 15, 2019 •

edited

Loading

codecov bot commented Mar 15, 2019 •

edited

Loading

karakayasemi commented Mar 16, 2019 •

edited

Loading