Default share perms #28619

PVince81 · 2017-08-08T11:10:04Z

Description

Default share permissions setting for local shares.

The default permissions are exposed via capabilities.
To make it visible to the JS frontend code, the capabilities are now added in "js/config.php" and accessible through OC.getCapabilities(). This will make passing such config values more convenient in the future especially for values that other clients need.

Related Issue

Fixes #28384.

Motivation and Context

See ticket

How Has This Been Tested?

TEST: manual testing
TEST: add unit tests

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.

PVince81 · 2017-09-01T09:59:39Z

@felixheidecke any hint to make it look a bit more beautiful ? Since there is no main checkbox I thought maybe to just add a bullet there ?

cc @pmaier1

PVince81 · 2017-09-01T09:59:52Z

talking about the "Default share permissions" section

Capabilities are computed in js/config.php and stored in oc_capabilities and accessible under OC.getCapabilities() on the JS side. This is only exposed if the user has logged in.

PVince81 · 2017-09-01T10:35:56Z

Alternative layout (not committed):

PVince81 · 2017-09-01T10:55:12Z

Test plan

Set default perms in settings page to "can create" and "can update" only

TEST: creating a user share in web UI uses default perm
TEST: capability API returns default perm 7
TEST: create a user share with OCS API without giving a permission value: curl -u admin:admin -H 'Content-Type:application/x-www-form-urlencoded' -H 'OCS-APIREQUEST:true' -X POST 'http://localhost/owncloud/ocs/v1.php/apps/files_sharing/api/v1/shares' --data-binary 'path=/test&shareType=0&shareWith=user3' => result returns default perm instead of 31, web UI shows that the perm was applied correctly

PVince81 · 2017-09-01T12:41:06Z

Oh, legit test failures on Jenkins. It's been a while I haven't seen that:

Tests\Settings\Panels\Admin\FileSharingTest.testGetSection
    Tests\Settings\Panels\Admin\FileSharingTest.testGetPriority
    Tests\Settings\Panels\Admin\FileSharingTest.testGetPanelEnabled
    OCA\Files_Sharing\Tests\ApiTest::testPublicLinkExpireDate.testPublicLinkExpireDate with data set #0
    OCA\Files_Sharing\Tests\ApiTest::testPublicLinkExpireDate.testPublicLinkExpireDate with data set #1
    OCA\Files_Sharing\Tests\ApiTest::testPublicLinkExpireDate.testPublicLinkExpireDate with data set #2
    OCA\Files_Sharing\Tests\ApiTest.testCreateShareUserFile
    OCA\Files_Sharing\Tests\ApiTest.testCreateShareUserFolder
    OCA\Files_Sharing\Tests\ApiTest.testCreateShareGroupFile
    OCA\Files_Sharing\Tests\ApiTest.testCreateShareGroupFolder
    OCA\Files_Sharing\Tests\ApiTest.testCreateShareLink
    OCA\Files_Sharing\Tests\ApiTest.testCreateShareLinkPublicUpload
    OCA\Files_Sharing\Tests\ApiTest.testEnfoceLinkPassword
    OCA\Files_Sharing\Tests\ApiTest.testSharePermissions
    OCA\Files_Sharing\Tests\ApiTest.testGetAllShares
    OCA\Files_Sharing\Tests\ApiTest.testGetAllSharesWithMe
    OCA\Files_Sharing\Tests\ApiTest.testPublicLinkUrl
    OCA\Files_Sharing\Tests\ApiTest.testGetShareFromFolder
    OCA\Files_Sharing\Tests\ApiTest.testGetShareFromFolderWithFile
    OCA\Files_Sharing\Tests\ApiTest.testGetShareFromFolderReshares
    OCA\Files_Sharing\Tests\ApiTest.testGetShareFromSubFolderReShares
    OCA\Files_Sharing\Tests\ApiTest.testGetShareFromFolderReReShares
    OCA\Files_Sharing\Tests\ApiTest.testGetShareMultipleSharedFolder
    OCA\Files_Sharing\Tests\ApiTest.testGetShareFromFileReReShares
    OCA\Files_Sharing\Tests\ApiTest.testGetShareFromUnknownId
    OCA\Files_Sharing\Tests\ApiTest.testUpdateShareUpload
    OCA\Files_Sharing\Tests\ApiTest.testUpdateShareExpireDate
    OCA\Files_Sharing\Tests\ApiTest.testDeleteReshare
    OCA\Files_Sharing\Tests\ApiTest.testDefaultExpireDate
    OCA\Files_Sharing\Tests\ApiTest.testCreatePublicLinkExpireDateValid
    OCA\Files_Sharing\Tests\ApiTest.testCreatePublicLinkExpireDateInvalidFuture
    OCA\Files_Sharing\Tests\ApiTest.testCreatePublicLinkExpireDateInvalidPast
    OCA\Files_Sharing\Tests\ApiTest.testInvisibleSharesUser
    OCA\Files_Sharing\Tests\ApiTest.testInvisibleSharesGroup
    OCA\Files_Sharing\Tests\ExpireSharesJobTest::testExpireLinkShare.testExpireLinkShare with data set #0
    OCA\Files_Sharing\Tests\ExpireSharesJobTest::testExpireLinkShare.testExpireLinkShare with data set #1
    OCA\Files_Sharing\Tests\ExpireSharesJobTest::testExpireLinkShare.testExpireLinkShare with data set #2
    OCA\Files_Sharing\Tests\ExpireSharesJobTest::testExpireLinkShare.testExpireLinkShare with data set #3
    OCA\Files_Sharing\Tests\ExpireSharesJobTest::testExpireLinkShare.testExpireLinkShare with data set #4
    OCA\Files_Sharing\Tests\ExpireSharesJobTest::testExpireLinkShare.testExpireLinkShare with data set #5
    OCA\Files_Sharing\Tests\ExpireSharesJobTest::testExpireLinkShare.testExpireLinkShare with data set #6
    OCA\Files_Sharing\Tests\ExpireSharesJobTest::testExpireLinkShare.testExpireLinkShare with data set #7
    OCA\Files_Sharing\Tests\ExpireSharesJobTest::testExpireLinkShare.testExpireLinkShare with data set #8
    OCA\Files_Sharing\Tests\ExpireSharesJobTest::testExpireLinkShare.testExpireLinkShare with data set #9

jvillafanez · 2017-09-01T12:48:58Z

👍 for the code. We still need to check the failing unittests and the style

pmaier1 · 2017-09-02T15:51:50Z

Nice! Some UI feedback:

IMO it would fit best at the bottom of the sharing section. This would also make the missing main checkbox less noticeable. Why did you put it in the middle? For that I would also maybe prefer the second layout.
In the headline, I think the term 'local' could be confusing. It could say 'Default user and group share permissions'?
Consistency: The checkbox for 'update' is called 'change' in the sharing tab. In general I find that one confusing because if I can change it, I can also delete it's contents but that's another story ;)

tomneedham · 2017-09-04T08:40:46Z

UI wise, I prefer the uncommitted proposed design, but at the bottom of the sharing settings

PVince81 · 2017-09-04T13:15:04Z

the uncommitted proposed design

@tomneedham they have swapped roles. The committed one is now the one with the aligned checkboxes on a single row.

PVince81 · 2017-09-04T13:19:40Z

Adjusted according to @pmaier1's comments.

PVince81 · 2017-09-04T13:20:36Z

PVince81 · 2017-09-04T16:03:29Z

stable10: #28903

(am optimistic about CI)

lock · 2019-08-02T15:01:43Z

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.

PVince81 added the 2 - Developing label Aug 8, 2017

PVince81 added this to the development milestone Aug 8, 2017

PVince81 self-assigned this Aug 8, 2017

PVince81 mentioned this pull request Aug 8, 2017

Make sharing permission defaults configurable #28384

Closed

PVince81 added the p1-urgent Critical issue, need to consider hotfix with just that issue label Sep 1, 2017

Vincent Petry added 4 commits September 1, 2017 12:30

Expose capabilities in core JS

0805ef5

Capabilities are computed in js/config.php and stored in oc_capabilities and accessible under OC.getCapabilities() on the JS side. This is only exposed if the user has logged in.

Add setting for default share permissions

4527cd5

Details default share permissions in settings page

8e0bbe4

OCS API now uses default share perms if none passed at creation time

a186fa1

PVince81 force-pushed the default-share-perms branch from 9e5d9be to a186fa1 Compare September 1, 2017 10:36

Vincent Petry added 2 commits September 1, 2017 12:44

Improve default share perms section layout

56c0634

Fix share capabilities JS tests

7674147

PVince81 added 3 - To Review and removed 2 - Developing labels Sep 1, 2017

PVince81 requested review from IljaN, jvillafanez, lordelix and tomneedham September 1, 2017 10:55

IljaN approved these changes Sep 1, 2017

View reviewed changes

Fix share API unit tests due to default perms setting

d9178c8

Tweak layout and wording of defualt file share permissions

9e60e86

PVince81 added 4 - To release and removed 3 - To Review labels Sep 4, 2017

PVince81 mentioned this pull request Sep 4, 2017

[stable10] Default share perms #28903

Merged

PVince81 merged commit 916afbd into master Sep 4, 2017

PVince81 deleted the default-share-perms branch September 4, 2017 19:28

ownclouders added the status/STALE label Dec 20, 2017

lock bot locked as resolved and limited conversation to collaborators Aug 2, 2019

Default share perms #28619

Default share perms #28619

Uh oh!

Conversation

PVince81 commented Aug 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Uh oh!

PVince81 commented Sep 1, 2017

Uh oh!

PVince81 commented Sep 1, 2017

Uh oh!

PVince81 commented Sep 1, 2017

Uh oh!

PVince81 commented Sep 1, 2017

Test plan

Uh oh!

PVince81 commented Sep 1, 2017

Uh oh!

jvillafanez commented Sep 1, 2017

Uh oh!

pmaier1 commented Sep 2, 2017

Uh oh!

tomneedham commented Sep 4, 2017

Uh oh!

PVince81 commented Sep 4, 2017

Uh oh!

PVince81 commented Sep 4, 2017

Uh oh!

PVince81 commented Sep 4, 2017

Uh oh!

PVince81 commented Sep 4, 2017

Uh oh!

lock bot commented Aug 2, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

PVince81 commented Aug 8, 2017 •

edited

Loading