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

Add hidden user folder #29886

Closed
wants to merge 7 commits into from
Closed

Add hidden user folder #29886

wants to merge 7 commits into from

Conversation

icewind1991
Copy link
Member

this can be used by apps to have apps that are within the users home folder but are hidden from the normal ui bits.

Currently, the "hidden" part is achieved in the dav plugin excluding it from listing and file search excluding results inside it.

I had originally hoped to have the "hidden" logic be more low level instead of having to implement it separately in each api by my attempts for that didn't work out as they would defeat the purpose of the folder in the first place (having it accessible trough normal endpoints by path and fileid)

@icewind1991
Copy link
Member Author

added a function get generate the folder name and added a dot prefix

@skjnldsv skjnldsv requested review from PVince81, juliushaertl and a team November 29, 2021 15:22
@skjnldsv skjnldsv added 3. to review Waiting for reviews enhancement labels Nov 29, 2021
Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

A bit of tests would be awesome 🙈

@PVince81
Copy link
Member

PVince81 commented Nov 29, 2021

  • BUG: create a folder "test" in the web UI then rename it to the hidden folder => this should not be possible
  • BUG: rename hidden folder to another name => this should not be possible
  • BUG: Webdav DELETE on hidden folder did not fail, and I can even recreate it manually
  • I assume that a PROPFIND on the hidden folder should not be blocked ? it's not currently

there might be more

yeah, would be good to add maybe integration tests for this to confirm that all file operations are denied there or at least that the folder is protected in some way

@PVince81
Copy link
Member

PVince81 commented Nov 29, 2021

  • BUG: I shouldn't be able to share the hidden folder with someone else (per API)
  • BUG: while the folder is invisible in the web UI, if I try to create it, it will suddenly reappear with "could not create because it exists already" and suddenly I see it and can access it

@PVince81
Copy link
Member

PVince81 commented Nov 29, 2021

  • don't create activity entries for any entries inside the hidden folder ? for example if apps are creating custom folders and files there

@icewind1991
Copy link
Member Author

nextcloud/activity#698 for the activity path

I assume that a PROPFIND on the hidden folder should not be blocked ? it's not currently

This is intentional yes

while the folder is invisible in the web UI, if I try to create it, it will suddenly reappear with "could not create because it exists already" and suddenly I see it and can access it

I think this is ok, due to the "random" name it's very unlikely that a user hits this by accident, and being able to browse the hidden folder on purpose is fine since it's not a security feature.

Copy link
Contributor

@artonge artonge left a comment

Choose a reason for hiding this comment

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

From reading the code:

  • What if the user creates a folder with the same name as the hidden folder but in a subfolder.
/
/.hidden_azeudhazud    <-- real hidden folder
/sub_folder
/sub_folder/.hidden_azeudhazud    <-- user creates this folder that should maybe not be hidden.

If I get this right, the created folder will be considered as hidden by FileSystem::isPathHidden.

Is this intended behaviour?

It makes sense if userA share his root folder with userB

/    <-- userB's root folder
/.hidden_azeudhazud    <-- userB's hidden folder
/usera_shared_root_folder    <-- what the name says
/usera_shared_root_folder/.hidden_azeudhazud    <-- userA's hidden folder that should be hidden

lib/private/Files/Filesystem.php Outdated Show resolved Hide resolved
apps/dav/lib/Connector/Sabre/Directory.php Outdated Show resolved Hide resolved
lib/private/Files/Node/Folder.php Outdated Show resolved Hide resolved
lib/private/Files/Node/Root.php Outdated Show resolved Hide resolved
lib/private/Files/View.php Outdated Show resolved Hide resolved
lib/private/Files/Filesystem.php Fixed Show fixed Hide fixed
lib/private/Files/Filesystem.php Fixed Show fixed Hide fixed
apps/dav/lib/Files/HiddenFolderPlugin.php Fixed Show fixed Hide fixed
apps/dav/lib/Files/HiddenFolderPlugin.php Fixed Show fixed Hide fixed
apps/dav/lib/Files/HiddenFolderPlugin.php Fixed Show fixed Hide fixed

$hiddenName = Filesystem::getHiddenFolderName();
if ($share->getNode()->getName() === $hiddenName) {
$event->stopPropagation();

Check notice

Code scanning / Psalm

DeprecatedMethod

The method Symfony\Component\EventDispatcher\Event::stopPropagation has been marked as deprecated
$server->on('beforeUnbind', [$this, 'onBind'], 1000);
}

public function onBind($path): bool {

Check notice

Code scanning / Psalm

MissingParamType

Parameter $path has no provided type
this can be used by apps to have apps that are within the users home
folder but are hidden from the normal ui bits

Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
@skjnldsv skjnldsv added the pending documentation This pull request needs an associated documentation update label Jan 31, 2023
@skjnldsv skjnldsv requested a review from artonge January 31, 2023 10:15
@skjnldsv skjnldsv added this to the Nextcloud 28 milestone May 9, 2023
@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
This was referenced Nov 6, 2023
This was referenced Nov 14, 2023
@blizzz blizzz modified the milestones: Nextcloud 28, Nextcloud 29 Nov 23, 2023
@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Mar 10, 2024
This was referenced Mar 12, 2024
This was referenced Mar 20, 2024
@skjnldsv skjnldsv mentioned this pull request Mar 28, 2024
81 tasks
@skjnldsv skjnldsv modified the milestones: Nextcloud 29, Nextcloud 30 Mar 28, 2024
@juliushaertl juliushaertl removed their request for review July 15, 2024 13:39
This was referenced Jul 30, 2024
@Altahrim Altahrim mentioned this pull request Aug 5, 2024
@skjnldsv skjnldsv modified the milestones: Nextcloud 30, Nextcloud 31 Aug 6, 2024
@skjnldsv skjnldsv closed this Aug 14, 2024
@skjnldsv skjnldsv removed this from the Nextcloud 31 milestone Aug 14, 2024
@susnux susnux deleted the hidden-folder branch August 26, 2024 00:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress enhancement pending documentation This pull request needs an associated documentation update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants