Exclude Directories by mmattel · Pull Request #19235 · owncloud/core

mmattel · 2015-09-22T07:19:30Z

Successor of PR #16534

Before mmattel/core
Now owncloud/core

Shoud fix CI start

schiessle · 2015-09-22T08:06:40Z

apps/files/ajax/newfile.php

shouldn't the hook catch this already?
https://github.com/owncloud/core/pull/19235/files#diff-73d71f74c9f6d7093f369006c0c60dfeR585

No it does not.
And we need this query here to display on the UI the text.

What's about "upload" then? There you also get a generic error message only, right?
Maybe it would be a good idea to let the hook throw an exception with the corresponding error message if it is forbidden and then make sure that we display the error message from the exception instead of a generic one. You could use the HintException to provide a translated message. We do something similar with encryption errors.

It is probably more failure save instead of adding if statements all over the place and it is more generic since other errors can be handled the same way.

oparoz · 2015-09-25T09:43:57Z

@mmattel @PVince81 @rperezb In order to test this properly, could we have some sort of acceptance test list with checkboxes? Would make it easier to know what's been tested and which corner cases we may have forgotten about.

mmattel · 2015-09-26T09:37:37Z

Testscenario_1:
Focus: try to create a excluded directory name
Preperation: the name must NOT exist in the filesystem

Storage	Task (t)	Browser	Webdav	Creatable	Comment
local	D	x		yes
local	E	x		no	(1)
local	D		x	yes
local	E		x	no	(2)
SMB	D	x		yes
SMB	E	x		no	(1)
SMB	D		x	yes
SMB	E		x	no	(2)

t ... define a directory name to be excluded like fgh
- E ... enable it config.php
- D ... disable it in config.php (I just renamed it to .fgh and vice versa)
r ... y (yes), n (no), -- (n/a)

(1) ... A browser popup should appear that this name is excluded and can´t be used
(2) ... WebDav reports 'Forbidden'

Testscenario_1 sucessfully finished

Testscenario_2:
Focus: are the directories properly shown.
Preperation: have a directory already created with an excluded name which will then be checked against. This means, that the ecluded directory name already exists in the filesystem.

Storage	Task (t)	Browser	Webdav	Is_Visible
local	D	x		yes
local	E	x		no
local	D		x	yes
local	E		x	no
SMB	D	x		yes
SMB	E	x		no
SMB	D		x	yes
SMB	E		x	no

t ... define a directory name to be excluded like fgh
- E ... enable it config.php
- D ... disable it in config.php (I just renamed it to .fgh and vice versa)
r ... y (yes), n (no), -- (n/a)
Testscenario_2 sucessfully finished

Testscenario_3:
Focus: try to create an excluded directory name where the name DOES already exist in the filesystem
Preperation: like test 2

Storage	Task (t)	Browser	Webdav	Is_Creatable	Comment
local	D	x		no	same as Test2
local	E	x		no	same as Test2
local	D		x	no	(1)
local	E		x	no	(1)
SMB	D	x		no	same as Test2
SMB	E	x		no	same as Test2
SMB	D		x	no	(1)
SMB	E		x	no	(1)

t ... define a directory name to be excluded like fgh
- E ... enable it config.php
- D ... disable it in config.php (I just renamed it to .fgh and vice versa)
r ... y (yes), n (no), -- (n/a)

(1) ... A Fatal Error Message is created, the WebDav client also shows an error. But this is according to my tests not related to this PR. For details pls see #19101 (WebDav: create directory with an already existing directory name -> Fatal error)

Testscenario_3 sucessfully finished

Note : I wanted to use checkboxes in the table, but this did not work

oparoz · 2015-09-26T10:07:26Z

Excellent, thank you :)

MorrisJobke · 2015-09-28T09:36:31Z

@DeepDiver1975 @oparoz I don't think we want this in 8.2. We are already in feature freeze.

oparoz · 2015-09-28T10:21:42Z

I don't have a say in this ;)

MorrisJobke · 2015-09-28T10:49:34Z

@DeepDiver1975 @cmonteroluque enhancement -> 9.0

ghost · 2015-09-30T18:18:11Z

@MorrisJobke ok

DeepDiver1975 · 2015-11-10T15:19:03Z

rebased

PVince81 · 2016-01-08T10:12:53Z

@icewind1991 can you please comment on this PR ?

DeepDiver1975 · 2016-01-13T15:17:39Z

needs rebase

ghost · 2016-01-16T00:52:11Z

Please also ping in owncloud-archive/documentation#1275 if this PR gets merged.

PVince81 · 2016-08-22T18:52:22Z

Rebased, not tested yet (files app still seems to work at the moment)

DeepDiver1975 · 2016-08-23T08:16:26Z

unit tests are badly falling apart - 89 failed tests
https://jenkins.owncloud.org/job/owncloud-core/job/core/job/PR-19235/1/testReport/

mmattel · 2016-08-23T10:44:02Z

lib/private/Files/View.php

 			 */
+			$files = array_filter(function(ICacheEntry $content) {
+				return (!\OC\Files\Filesystem::isForbiddenFileOrDir($content['path']));
+			});


Should´nt that be (without testing):

$files = array_filter($content, function(ICacheEntry $c) { return (!\OC\Files\Filesystem::isForbiddenFileOrDir($c['path'])); });

PVince81 · 2016-08-24T08:17:42Z

Did a quick test with the old Webdav and new Webdav endpoints and an excluded folder called "snapshots".

Seems to work so far: listing, deleting or overwriting the folder is denied.

mmattel · 2016-08-24T16:31:35Z

Great !
I have written test cases, pls see above. If they pass then this PR should be ready. Sorry if I cant do, businesstrip...

PVince81 · 2016-08-24T17:47:34Z

businesstrip...

No time to kill on the plane ? (if applicable) 😉

Am thinking of maybe writing integration tests to automatically test this.
The browser test cases can be removed as the web UI uses Webdav already since 9.0 (except for uploads)

extended case: search at any place of the path given adding case insensitiveness added suggestions and improvements removed unnecessary function parameter new approach according the rethinking fixed code and unit tests update comments improved code, added calls for trashbin copyFromStorage

mmattel · 2016-09-02T09:50:27Z

@PVince81 any news? would really appreciate to get that in

PVince81 · 2016-09-02T10:16:42Z

Tested with many file operations on local storage and external storage and also subdirectories.
Sync client works too.
Any attempt to do any file operation on the excluded folder results in 403 Forbidden.

Exclusion works fine 👍

PVince81 · 2016-09-02T10:20:36Z

added to 9.2 feature list: https://github.com/owncloud/core/wiki/ownCloud-10.0-Features
added doc ticket: [9.2] Setting to exclude folders owncloud-archive/documentation#2603

PVince81 · 2016-09-02T10:20:49Z

@mmattel thanks a lot ! And sorry that it took so long to get in.

mmattel · 2016-09-02T11:04:15Z

🎉 😃

… Storage use) owncloud/core/pull/19235 Adds the ability to define directories in config.php which will not be further processed (scanned, shown, created ect). These directories exist but will not handled in nextcloud. Usecase: eg. filesystem which have the ability of snapshots have a defined naming for them. For details pls see also the documentation PR coming asap.

JKawohl · 2017-02-22T13:50:01Z

@PVince81 is this safe to patch into current 9.1 ?

PVince81 · 2017-02-23T09:28:59Z

@kawohl no, this is a huge change and I wouldn't recommend patching. This would need careful testing.

JKawohl · 2017-02-23T09:44:41Z

thanks!

lock · 2019-08-03T21:02:38Z

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.

schiessle reviewed Sep 22, 2015
View reviewed changes

DeepDiver1975 added the 3 - To Review label Sep 22, 2015

mmattel force-pushed the exclude_directories_III branch from 89e0238 to 7a0268b Compare September 22, 2015 15:52

PVince81 mentioned this pull request Sep 25, 2015

ignore system directories within webinterface #19325

Closed

oparoz added this to the 8.2-current milestone Sep 25, 2015

oparoz added the enhancement label Sep 25, 2015

oparoz mentioned this pull request Sep 26, 2015

enhancement idea: Global sync-exclude.lst #19385

Closed

MorrisJobke modified the milestones: 9.0-next, 8.2-current Sep 28, 2015

ghost mentioned this pull request Oct 15, 2015

excluded directories and blacklisted files owncloud-archive/documentation#1275

Closed

DeepDiver1975 force-pushed the exclude_directories_III branch from 7a0268b to b5b954b Compare November 10, 2015 15:17

MorrisJobke added resolves-blue-ticket and removed resolves-blue-ticket labels Nov 10, 2015

PVince81 added the comp:filesystem label Nov 17, 2015

PVince81 added the resolves-blue-ticket label Jan 7, 2016

MorrisJobke added blue-ticket and removed resolves-blue-ticket labels Jan 7, 2016

LukasReschke assigned icewind1991 Feb 5, 2016

mmattel reviewed Aug 23, 2016
View reviewed changes

mmattel and others added 4 commits August 29, 2016 14:19

fix local storage

40b92bc

Fix rebase issue in view

b234170

DeepDiver1975 force-pushed the exclude_directories_III branch from f8cd27a to b234170 Compare August 29, 2016 12:19

PVince81 merged commit 1826836 into master Sep 2, 2016

PVince81 deleted the exclude_directories_III branch September 2, 2016 10:16

PVince81 mentioned this pull request Sep 2, 2016

[9.2] Setting to exclude folders owncloud-archive/documentation#2603

Closed

mmattel mentioned this pull request Sep 25, 2016

Exlude particular directories from beeing viewed, accessed and scanned #15385

Closed

mmattel mentioned this pull request Oct 5, 2016

Exclude Directories nextcloud/server#1627

Closed

PVince81 mentioned this pull request Oct 10, 2016

Add configuration option to exclude directories and files from file system scan #10792

Closed

PVince81 mentioned this pull request Apr 26, 2017

Update release_notes.rst owncloud-archive/documentation#3024

Merged

mmattel mentioned this pull request Sep 14, 2017

Prevent .part upload via dav #29011

Merged

9 tasks

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

Conversation

mmattel commented Sep 22, 2015

Uh oh!

schiessle Sep 22, 2015

Choose a reason for hiding this comment

Uh oh!

mmattel Sep 22, 2015

Choose a reason for hiding this comment

Uh oh!

schiessle Sep 28, 2015

Choose a reason for hiding this comment

Uh oh!

oparoz commented Sep 25, 2015

Uh oh!

mmattel commented Sep 26, 2015

Uh oh!

oparoz commented Sep 26, 2015

Uh oh!

MorrisJobke commented Sep 28, 2015

Uh oh!

oparoz commented Sep 28, 2015

Uh oh!

MorrisJobke commented Sep 28, 2015

Uh oh!

ghost commented Sep 30, 2015

Uh oh!

DeepDiver1975 commented Nov 10, 2015

Uh oh!

PVince81 commented Jan 8, 2016

Uh oh!

DeepDiver1975 commented Jan 13, 2016

Uh oh!

ghost commented Jan 16, 2016

Uh oh!

PVince81 commented Aug 22, 2016

Uh oh!

DeepDiver1975 commented Aug 23, 2016

Uh oh!

mmattel Aug 23, 2016

Choose a reason for hiding this comment

Uh oh!

PVince81 Aug 24, 2016

Choose a reason for hiding this comment

Uh oh!

PVince81 commented Aug 24, 2016

Uh oh!

mmattel commented Aug 24, 2016

Uh oh!

PVince81 commented Aug 24, 2016

Uh oh!

mmattel commented Sep 2, 2016

Uh oh!

PVince81 commented Sep 2, 2016

Uh oh!

PVince81 commented Sep 2, 2016 • edited by hodyroff Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PVince81 commented Sep 2, 2016

Uh oh!

mmattel commented Sep 2, 2016

Uh oh!

JKawohl commented Feb 22, 2017 • edited by DeepDiver1975 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PVince81 commented Feb 23, 2017

Uh oh!

JKawohl commented Feb 23, 2017

Uh oh!

lock bot commented Aug 3, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

PVince81 commented Sep 2, 2016 •

edited by hodyroff

Loading

JKawohl commented Feb 22, 2017 •

edited by DeepDiver1975

Loading