Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Better check reshare permissions part2 #16186

Merged
merged 3 commits into from Jul 3, 2019
Merged

Better check reshare permissions part2 #16186

merged 3 commits into from Jul 3, 2019

Conversation

nickvergessen
Copy link
Member

Signed-off-by: Joas Schilling coding@schilljs.com

@nickvergessen nickvergessen added this to the Nextcloud 17 milestone Jul 2, 2019
ChristophWurst
ChristophWurst approved these changes Jul 2, 2019
View reviewed changes
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code makes sense 👍

@nickvergessen
Copy link
Member Author

/backport to stable16

@nickvergessen
Copy link
Member Author

/backport to stable15

@nickvergessen
Copy link
Member Author

/backport to stable14

@MorrisJobke MorrisJobke force-pushed the bugfix/noid/also-check-permissions-when-creating-a-share branch from 83e40df to ac4e752 Compare July 2, 2019 14:40
@MorrisJobke
Copy link
Member

Failures are due to #16198

@MorrisJobke MorrisJobke added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jul 2, 2019
@MorrisJobke MorrisJobke force-pushed the bugfix/noid/also-check-permissions-when-creating-a-share branch from ac4e752 to 99d1769 Compare July 2, 2019 17:22
@rullzer
Copy link
Member

rullzer commented Jul 2, 2019

Ai more failing tests... https://drone.nextcloud.com/nextcloud/server/19714/14/4

@MorrisJobke
Copy link
Member 


631	Scenario: User is not allowed to reshare file with more permissions # /drone/src/build/integration/features/sharing-v1-part2.feature:234
632	As an "admin"
633	[Wed Jul 3 07:59:37 2019] 127.0.0.1:55796 [404]: /ocs/v2.php/cloud/users/user0
634	[Wed Jul 3 07:59:37 2019] 127.0.0.1:55802 [200]: /ocs/v1.php/cloud/users
635	[Wed Jul 3 07:59:37 2019] 127.0.0.1:55810 [200]: /ocs/v1.php/cloud/users/user0
636	[Wed Jul 3 07:59:38 2019] 127.0.0.1:55814 [200]: /ocs/v2.php/cloud/users/user0
637	Given user "user0" exists # FeatureContext::assureUserExists()
638	[Wed Jul 3 07:59:38 2019] 127.0.0.1:55818 [404]: /ocs/v2.php/cloud/users/user1
639	[Wed Jul 3 07:59:38 2019] 127.0.0.1:55820 [200]: /ocs/v1.php/cloud/users
640	[Wed Jul 3 07:59:38 2019] 127.0.0.1:55826 [200]: /ocs/v1.php/cloud/users/user1
641	[Wed Jul 3 07:59:38 2019] 127.0.0.1:55830 [200]: /ocs/v2.php/cloud/users/user1
642	And user "user1" exists # FeatureContext::assureUserExists()
643	[Wed Jul 3 07:59:38 2019] 127.0.0.1:55834 [404]: /ocs/v2.php/cloud/users/user2
644	[Wed Jul 3 07:59:39 2019] 127.0.0.1:55836 [200]: /ocs/v1.php/cloud/users
645	[Wed Jul 3 07:59:39 2019] 127.0.0.1:55842 [200]: /ocs/v1.php/cloud/users/user2
646	[Wed Jul 3 07:59:39 2019] 127.0.0.1:55846 [200]: /ocs/v2.php/cloud/users/user2
647	And user "user2" exists # FeatureContext::assureUserExists()
648	And As an "user0" # FeatureContext::asAn()
649	[Wed Jul 3 07:59:39 2019] 127.0.0.1:55850 [200]: /ocs/v1.php/apps/files_sharing/api/v1/shares
650	And creating a share with # FeatureContext::creatingShare()
651	| path | /textfile0.txt |
652	| shareType | 0 |
653	| shareWith | user1 |
654	| permissions | 16 |
655	And As an "user1" # FeatureContext::asAn()
656	[Wed Jul 3 07:59:39 2019] 127.0.0.1:55856 [200]: /ocs/v1.php/apps/files_sharing/api/v1/shares
657	When creating a share with # FeatureContext::creatingShare()
658	| path | /textfile0 (2).txt |
659	| shareType | 0 |
660	| shareWith | user2 |
661	| permissions | 31 |
662	Then the OCS status code should be "404" # FeatureContext::theOCSStatusCodeShouldBe()
663	Failed asserting that SimpleXMLElement Object &0000000072b4142c00000000301d57f1 (
664	0 => '100'
665	) matches expected '404'.
666	And the HTTP status code should be "200" # FeatureContext::theHTTPStatusCodeShouldBe()
667	[Wed Jul 3 07:59:39 2019] 127.0.0.1:55860 [200]: /ocs/v1.php/cloud/users/user0
668	[Wed Jul 3 07:59:39 2019] 127.0.0.1:55866 [404]: /ocs/v2.php/cloud/users/user0
669	[Wed Jul 3 07:59:40 2019] 127.0.0.1:55868 [200]: /ocs/v1.php/cloud/users/user1
670	[Wed Jul 3 07:59:40 2019] 127.0.0.1:55874 [404]: /ocs/v2.php/cloud/users/user1
671	[Wed Jul 3 07:59:40 2019] 127.0.0.1:55876 [200]: /ocs/v1.php/cloud/users/user2
672	[Wed Jul 3 07:59:40 2019] 127.0.0.1:55890 [404]: /ocs/v2.php/cloud/users/user2
673	[Wed Jul 3 07:59:40 2019] Login failed: 'user0' (Remote IP: '127.0.0.1')
674	[Wed Jul 3 07:59:40 2019] 127.0.0.1:55894 [401]: /remote.php/webdav/myFileToComment.txt
675	[Wed Jul 3 07:59:41 2019] 127.0.0.1:55896 [207]: /remote.php/dav/systemtags/
676	[Wed Jul 3 07:59:41 2019] Login failed: 'user0' (Remote IP: '127.0.0.1')
677	[Wed Jul 3 07:59:41 2019] 127.0.0.1:55900 [401]: /remote.php/webdav/myFileToTag.txt
678	[Wed Jul 3 07:59:41 2019] 127.0.0.1:55902 [404]: /remote.php/dav/addressbooks/users/admin/MyAddressbook
679	[Wed Jul 3 07:59:41 2019] 127.0.0.1:55906 [404]: /remote.php/dav/calendars/admin/MyCalendar

@nickvergessen
Better check reshare permissions when creating a share
e4addba 
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen nickvergessen force-pushed the bugfix/noid/also-check-permissions-when-creating-a-share branch from 32a46db to e4addba Compare July 3, 2019 12:01
@nickvergessen
Unify the permission checking in one place only
85a80b0 
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen
Remove duplicate tests
8783647 
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen
Copy link
Member Author

Unified the handling in the generalCreateChecks method now, instead of checking it on the API for create and update. This makes it more robust against invalid changes using a different future API which neglects this case by accident.

@MorrisJobke MorrisJobke merged commit c5c14d0 into master Jul 3, 2019
@delete-merged-branch delete-merged-branch bot deleted the bugfix/noid/also-check-permissions-when-creating-a-share branch July 3, 2019 18:07
@MorrisJobke
Copy link
Member

👍 from me

@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Jul 3, 2019
Merged
@backportbot-nextcloud
Copy link

backport to stable16 in #16211

@backportbot-nextcloud
Copy link

The backport to stable15 failed. Please do this backport manually.

@backportbot-nextcloud
Copy link

The backport to stable14 failed. Please do this backport manually.

This was referenced Jul 3, 2019
Merged
Merged
@MorrisJobke
Copy link
Member

Backport are in #16215 and #16216

@kesselb kesselb mentioned this pull request Jan 25, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MorrisJobke MorrisJobke Awaiting requested review from MorrisJobke

@rullzer rullzer Awaiting requested review from rullzer

@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst

Assignees
No one assigned
Labels
4. to release Ready to be released and/or waiting for tests to finish bug feature: sharing
Projects
None yet
Milestone
Nextcloud 17
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@nickvergessen @MorrisJobke @rullzer @ChristophWurst