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.

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

[full-ci] feat: allow editing of public link shared single files #40264

Merged
merged 5 commits into from
Aug 12, 2022

Conversation

DeepDiver1975
Copy link
Member

@DeepDiver1975 DeepDiver1975 commented Aug 3, 2022

Description

Add option to share a single file via public link with write permissions

Related Issue

https://github.com/owncloud/enterprise/issues/5274

How Has This Been Tested?

  • acceptance tests in CI
  • manual creation of public editable file link, and checking with the API

Note: there is no web UI to make use of this directly - when browsing to the public link, the standard page for viewing and downloading is shown. The purpose of this enhancement is to allow other clients to be able to edit single files in public links.

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:

@DeepDiver1975
Copy link
Member Author

Would be good to get some acceptance test coverage on this ..... @phil-davis THX

@phil-davis phil-davis self-assigned this Aug 3, 2022
@phil-davis
Copy link
Contributor

Would be good to get some acceptance test coverage on this ..... @phil-davis THX

I will look in the morning.

@phil-davis
Copy link
Contributor

https://drone.owncloud.com/owncloud/core/36485/9/5

Firefox 103.0 (Ubuntu 0.0.0): Executed 1044 of 1046 (5 FAILED) (skipped 2) (18.099 secs / 17.734 secs)

make: *** [Makefile:203: test-js] Error 1

test-javascript failed - @DeepDiver1975 I will let you adjust those tests.

@phil-davis
Copy link
Contributor

https://drone.owncloud.com/owncloud/core/36485/10/7

There were 5 failures:
...

@DeepDiver1975 - I will let you sort out the PHP unit tests.

@phil-davis
Copy link
Contributor

The permissions handling seems a bit strange:

  1. If I ask for 15, then I get 3:
curl -v -u aaaa:aaaa -X POST http://172.17.0.1:8080/ocs/v2.php/apps/files_sharing/api/v1/shares --data 'path=/t.txt&shareType=3&permissions=15&name=EditablePublicLink'

<?xml version="1.0"?>
<ocs>
 <meta>
  <status>ok</status>
  <statuscode>200</statuscode>
  <message/>
  <totalitems></totalitems>
  <itemsperpage></itemsperpage>
 </meta>
 <data>
  <id>1581</id>
  <share_type>3</share_type>
  <uid_owner>aaaa</uid_owner>
  <displayname_owner>Andrew Jones</displayname_owner>
  <permissions>3</permissions>
  <stime>1659687702</stime>
  <parent/>
  <expiration/>
  <token>RqOfpmsscTjwQK9</token>
  <uid_file_owner>aaaa</uid_file_owner>
  <displayname_file_owner>Andrew Jones</displayname_file_owner>
  <additional_info_owner/>
  <additional_info_file_owner/>
  <path>/t.txt</path>
  <mimetype>text/plain</mimetype>
  <storage_id>home::aaaa</storage_id>
  <storage>3238</storage>
  <item_type>file</item_type>
  <item_source>2147508369</item_source>
  <file_source>2147508369</file_source>
  <file_parent>2147506130</file_parent>
  <file_target>/t.txt</file_target>
  <name>EditablePublicLink</name>
  <url>http://172.17.0.1:8080/index.php/s/RqOfpmsscTjwQK9</url>
  <mail_send>0</mail_send>
  <attributes/>
 </data>
</ocs>
  1. If just ask for permissions 3, then I only get 1:
curl -v -u aaaa:aaaa -X POST http://172.17.0.1:8080/ocs/v2.php/apps/files_sharing/api/v1/shares --data 'path=/t.txt&shareType=3&permissions=3&name=TestPublicLink'

<?xml version="1.0"?>
<ocs>
 <meta>
  <status>ok</status>
  <statuscode>200</statuscode>
  <message/>
  <totalitems></totalitems>
  <itemsperpage></itemsperpage>
 </meta>
 <data>
  <id>1582</id>
  <share_type>3</share_type>
  <uid_owner>aaaa</uid_owner>
  <displayname_owner>Andrew Jones</displayname_owner>
  <permissions>1</permissions>
  <stime>1659687798</stime>
  <parent/>
  <expiration/>
  <token>BDpLdhEFZmAoY8R</token>
  <uid_file_owner>aaaa</uid_file_owner>
  <displayname_file_owner>Andrew Jones</displayname_file_owner>
  <additional_info_owner/>
  <additional_info_file_owner/>
  <path>/t.txt</path>
  <mimetype>text/plain</mimetype>
  <storage_id>home::aaaa</storage_id>
  <storage>3238</storage>
  <item_type>file</item_type>
  <item_source>2147508369</item_source>
  <file_source>2147508369</file_source>
  <file_parent>2147506130</file_parent>
  <file_target>/t.txt</file_target>
  <name>TestPublicLink</name>
  <url>http://172.17.0.1:8080/index.php/s/BDpLdhEFZmAoY8R</url>
  <mail_send>0</mail_send>
  <attributes/>
 </data>
</ocs>

The webUI is asking for permissions 15, and getting back permissions 3.

@DeepDiver1975
Copy link
Member Author

The permissions handling seems a bit strange:

This behavior did not really change in this PR.

Before this change when requesting 3 one would only get 1 in the response

@phil-davis
Copy link
Contributor

Before this change when requesting 3 one would only get 1 in the response

Yes, before this change, requesting anything other than 1 resulted in 1 or an error.

So, I guess you want to keep the old behavior that requesting 3 returned permission 1 only - so that any existing clients that are "silly" and currently routinely request "3" will still get the same behavior.

@phil-davis
Copy link
Contributor

The code in #40269 should be passing now. I made a few comments there. The test scenario at the API level works. The webUI works manually when I use it to create Download/View/Edit public links (I can add a UI test for that "whenever" early next week)

But when I browse in a private window to a Download/View/Edit public link I just get a page that lets me download. I can upload with a curl command. But is there (going to be) a UI interface that lets me:

  1. open the files_texteditor to edit a text file
  2. upload to the file (choose a file on my local system and upload it, which would completely overwrite the file on the server)

(2) might be a bit tricky - the file on the server might be a LibreOffice document myDocument.odt, for example, but then the person accessing the public link uploads some png file, and now myDocument.odt actually has binary data in it that is a png image. Letting a user choose from any local file to overwrite some fixed filename on the server could have painful results.

@DeepDiver1975
Copy link
Member Author

But when I browse in a private window to a Download/View/Edit public link I just get a page that lets me download. I can upload with a curl command. But is there (going to be) a UI interface that lets me:

1. open the files_texteditor to edit a text file

2. upload to the file (choose a file on my local system and upload it, which would completely overwrite the file on the server)

(2) might be a bit tricky - the file on the server might be a LibreOffice document myDocument.odt, for example, but then the person accessing the public link uploads some png file, and now myDocument.odt actually has binary data in it that is a png image. Letting a user choose from any local file to overwrite some fixed filename on the server could have painful results.

for the time being collabora is the only reasonable use case - we will enhance this over time.
for testing curl is fine

@phil-davis
Copy link
Contributor

or the time being collabora is the only reasonable use case - we will enhance this over time. for testing curl is fine

OK - I will make the UI test scenario use direct API requests to verify that the Download/View/Edit link works after being created in the UI.

@phil-davis
Copy link
Contributor

@DeepDiver1975 I have all the test code working in PR #40269

Can I rebase this PR and cherry-pick the test code into here?

Or you can do it.

The only code change that I need to make was 430abcb - please have a look. After that, all tests pass.

@DeepDiver1975
Copy link
Member Author

The only code change that I need to make was 430abcb - please have a look. After that, all tests pass.

look good - thx

@DeepDiver1975
Copy link
Member Author

Can I rebase this PR and cherry-pick the test code into here?

Or you can do it.

As you wish - I have only limited time available at the moment. THX

@phil-davis phil-davis force-pushed the feat/editable-public-link-file branch 2 times, most recently from 50e81be to df8fd0b Compare August 11, 2022 11:34
@phil-davis phil-davis force-pushed the feat/editable-public-link-file branch from df8fd0b to cc31f90 Compare August 11, 2022 11:43
@phil-davis phil-davis self-requested a review August 11, 2022 11:45
@owncloud owncloud deleted a comment from update-docs bot Aug 11, 2022
@owncloud owncloud deleted a comment from ownclouders Aug 11, 2022
@ownclouders
Copy link
Contributor

💥 Acceptance tests pipeline cliMain-mariadb10.2-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/36553/122

@phil-davis phil-davis changed the title feat: allow editing of public link shared single files [full-ci] feat: allow editing of public link shared single files Aug 11, 2022
@phil-davis phil-davis force-pushed the feat/editable-public-link-file branch from 3d9bdef to 91b9c31 Compare August 11, 2022 13:50
@phil-davis phil-davis force-pushed the feat/editable-public-link-file branch from 91b9c31 to 8b92375 Compare August 11, 2022 15:16
@sonarcloud
Copy link

sonarcloud bot commented Aug 11, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants