Add public-files to DAV by DeepDiver1975 · Pull Request #29369 · owncloud/core

DeepDiver1975 · 2017-10-26T19:54:21Z

Description

Add webdav resource for public shared files and folders

Motivation and Context

DAV rules!

How Has This Been Tested?

share a file or folder publicly
open the dav url /remote.php/dav/public-files/${token}

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.

DeepDiver1975 · 2017-10-26T20:14:14Z

deleting a file does not work

codecov · 2017-10-26T22:47:03Z

Codecov Report

Merging #29369 into master will decrease coverage by 0.47%.
The diff coverage is 11.62%.

@@             Coverage Diff              @@
##             master   #29369      +/-   ##
============================================
- Coverage     62.27%   61.79%   -0.48%     
- Complexity    18229    19112     +883     
============================================
  Files          1141     1096      -45     
  Lines         68119    61616    -6503     
  Branches       1230        0    -1230     
============================================
- Hits          42420    38077    -4343     
+ Misses        25336    23539    -1797     
+ Partials        363        0     -363

Flag	Coverage Δ	Complexity Δ
#javascript	`?`	`?`
#phpunit	`?`	`?`

Impacted Files	Coverage Δ	Complexity Δ
apps/dav/lib/DAV/PublicAuth.php	`18.18% <ø> (ø)`	`5 <0> (ø)`	⬇️
apps/dav/lib/Files/PublicFiles/ShareNode.php	`0% <0%> (ø)`	`8 <8> (?)`
lib/private/Files/Node/Node.php	`84.05% <0%> (-1.24%)`	`63 <1> (+1)`
apps/dav/lib/Files/PublicFiles/SharedFolder.php	`0% <0%> (ø)`	`13 <13> (?)`
apps/dav/lib/Files/PublicFiles/SharedFile.php	`0% <0%> (ø)`	`10 <10> (?)`
apps/dav/lib/RootCollection.php	`100% <100%> (ø)`	`1 <0> (ø)`	⬇️
apps/dav/lib/Server.php	`53.33% <100%> (+2.25%)`	`24 <0> (ø)`	⬇️
...ps/dav/lib/Files/PublicFiles/PublicSharingAuth.php	`27.77% <27.77%> (ø)`	`8 <8> (?)`
apps/dav/lib/Files/PublicFiles/RootCollection.php	`27.77% <27.77%> (ø)`	`7 <7> (?)`
lib/private/DB/PostgreSqlMigrator.php	`0% <0%> (-100%)`	`6% <0%> (ø)`
... and 191 more

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 df728bc...137b795. Read the comment docs.

PVince81

Code looks good so far, see some proposed additions.

PVince81 · 2017-11-17T09:47:19Z

apps/dav/lib/Files/PublicFiles/PublicSharingAuth.php

+	 * @inheritdoc
+	 */
+	function challenge(RequestInterface $request, ResponseInterface $response) {
+	}


if purposefully left empty, please add a comment stating so

PVince81 · 2017-11-17T09:47:39Z

apps/dav/lib/Files/PublicFiles/RootCollection.php

+
+	function __construct() {
+		$this->l10n = \OC::$server->getL10N('dav');
+		$this->shareManager = \OC::$server->getShareManager();


PVince81 · 2017-11-17T10:08:31Z

apps/dav/lib/Files/PublicFiles/ShareNode.php

+use Sabre\DAV\Collection;
+use Sabre\DAV\INode;
+
+class ShareNode extends Collection {


PHPDoc, what is this node about ?

PVince81 · 2017-11-17T10:09:06Z

apps/dav/lib/Files/PublicFiles/ShareNode.php

+		$this->share = $share;
+	}
+	/**
+	 * Returns an array with all the child nodes


I suggest clarifying and saying we are returning both SharedFolder and SharedFile nodes depending on child types

PVince81 · 2017-11-17T10:09:33Z

apps/dav/lib/Files/PublicFiles/SharedFile.php

+use Sabre\DAVACL\IACL;
+
+/**
+ * Class MetaFile


please adjust, smells like copy-pasted from another PR 😉

PVince81 · 2017-11-17T10:10:50Z

apps/dav/lib/Files/PublicFiles/SharedFile.php

+	}
+
+//	function setName($name) {
+//		$this->file->setName($name);


I think we can forbid renaming share file here. This is because the name itself isn't visible anyway but someone might attempt to hack the API to try it out.

For local shares the file name is a received mount point. But here for link shares this is no mount point.

Throw Forbidden ?

PVince81 · 2017-11-17T10:11:15Z

apps/dav/lib/Files/PublicFiles/SharedFolder.php

+use Sabre\DAVACL\IACL;
+
+/**
+ * Class MetaFolder


PVince81 · 2017-11-17T10:13:22Z

apps/dav/lib/Files/PublicFiles/SharedFolder.php

+		return $this->folder->getName();
+	}
+
+	function getLastModified() {


Please add getEtag() maybe.

We'll likely do a PROPFIND on this folder so it's good to get the etag to know if there were changes inside the folder.

Also add this to SharedFile.

Sounds like we might need a custom INode interface which has the getEtag method ? Or does Sabre retrieve the Etag differently ? (I forgot)

DeepDiver1975 · 2017-11-24T08:33:16Z

@ownclouders rebase

ownclouders · 2017-11-24T08:33:17Z

Hey! This pull request is being rebased automatically. Please DO NOT push while rebase is in progress or your changes would be lost!

ownclouders · 2017-11-24T08:33:33Z

Automated rebase failed! Please rebase your pull request manually via the command line.

Error:
Command '['git', 'rebase', 'base/master']' returned non-zero exit status 128.

PVince81 · 2018-06-28T13:13:14Z

Does it work with link password ?

This would obsolete the "public.php/webdav" endpoint which takes userid=$token and password=$password.

DeepDiver1975 · 2018-06-28T13:39:05Z

Does it work with link password ?

password handling in in here -
137b795

PVince81 · 2018-08-01T12:26:39Z

consider implications on federated shares and OCM. I expect that we only need to change the Webdav URL exposed in the discovery (to be tested)

PVince81 · 2018-10-09T14:38:26Z

reconsider for Phoenix ?

PVince81 · 2019-05-06T13:08:05Z

Fixes #23269

DeepDiver1975 · 2019-07-30T06:02:39Z

As discussed in #35777 the master branch will from now on hold the ownCloud 10 codebase.

This PR targetted ownCloud 11 which is postponed to a far distant future.

Because of that I'm closing this PR and kindly ask you to re-submit this PR in a few days.

Thanks a lot for your patience

DeepDiver1975 added the 3 - To Review label Oct 26, 2017

DeepDiver1975 added this to the development milestone Oct 26, 2017

DeepDiver1975 self-assigned this Oct 26, 2017

DeepDiver1975 requested review from PVince81 and butonic October 26, 2017 19:54

DeepDiver1975 force-pushed the dav-public-files branch from cbf7c73 to 8c0eb7b Compare October 26, 2017 21:30

PVince81 modified the milestones: development, planned Nov 6, 2017

PVince81 mentioned this pull request Nov 16, 2017

Enable chunking for bigger files in authenticated web upload #28415

Merged

1 task

PVince81 reviewed Nov 17, 2017

View reviewed changes

PVince81 modified the milestones: planned, development Nov 22, 2017

DeepDiver1975 force-pushed the dav-public-files branch from 389e677 to 078723b Compare November 24, 2017 08:49

ownclouders added the status/STALE label Dec 7, 2017

DeepDiver1975 force-pushed the dav-public-files branch from 078723b to 8376c96 Compare December 15, 2017 08:29

ownclouders removed the status/STALE label Dec 15, 2017

ownclouders added the status/STALE label Dec 23, 2017

PVince81 modified the milestones: development, planned Jan 12, 2018

DeepDiver1975 force-pushed the dav-public-files branch from 8376c96 to fc5590b Compare February 26, 2018 21:29

ownclouders removed the status/STALE label Feb 26, 2018

ownclouders added the status/STALE label Mar 6, 2018

DeepDiver1975 added 2 commits March 26, 2018 16:31

Add public-files to DAV including ACL handling

f3ab5b9

Add password verification for public shares

137b795

DeepDiver1975 force-pushed the dav-public-files branch from fc5590b to 137b795 Compare March 26, 2018 14:32

ownclouders removed the status/STALE label Mar 26, 2018

ownclouders added the status/STALE label Apr 3, 2018

felixboehm modified the milestones: development, planned Apr 10, 2018

DeepDiver1975 modified the milestones: development, triage Apr 25, 2018

PVince81 modified the milestones: triage, maybe some day May 22, 2018

ownclouders removed the status/STALE label Jun 28, 2018

ownclouders added the status/STALE label Jul 6, 2018

ownclouders removed the status/STALE label Aug 1, 2018

PVince81 added the v11 label Oct 9, 2018

DeepDiver1975 mentioned this pull request Oct 12, 2018

Public link share owncloud/web#270

Closed

PVince81 mentioned this pull request Feb 12, 2019

[stable10] Enable PreviewPlugin on public webdav #34472

Closed

11 tasks

PVince81 mentioned this pull request May 6, 2019

Consider new DAV endpoint for public shares #23269

Closed

DeepDiver1975 closed this Jul 30, 2019

DeepDiver1975 deleted the dav-public-files branch July 30, 2019 08:27

Conversation

DeepDiver1975 commented Oct 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

Uh oh!

DeepDiver1975 commented Oct 26, 2017

Uh oh!

codecov bot commented Oct 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

PVince81 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DeepDiver1975 commented Nov 24, 2017

Uh oh!

ownclouders commented Nov 24, 2017

Uh oh!

ownclouders commented Nov 24, 2017

Uh oh!

PVince81 commented Jun 28, 2018

Uh oh!

DeepDiver1975 commented Jun 28, 2018

Uh oh!

PVince81 commented Aug 1, 2018

Uh oh!

PVince81 commented Oct 9, 2018

Uh oh!

PVince81 commented May 6, 2019

Uh oh!

DeepDiver1975 commented Jul 30, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

DeepDiver1975 commented Oct 26, 2017 •

edited

Loading

codecov bot commented Oct 26, 2017 •

edited

Loading