Skip to content

feat: Add appconfig to disable fixed userfolder permissions optimization - #51145

Merged
artonge merged 3 commits into
masterfrom
home-folder-readonly
Oct 1, 2025
Merged

feat: Add appconfig to disable fixed userfolder permissions optimization#51145
artonge merged 3 commits into
masterfrom
home-folder-readonly

Conversation

@icewind1991

@icewind1991icewind1991 commented Feb 28, 2025•
edited by artonge
Loading

Copy link
Copy Markdown
Member

Currently, to save having to fetch the metadata of the home folder in many cases, we always assume the same value for the permissions of the home folder.

This does however break if the admin has configured an external storage mounted at / with different permissions.

To fix those cases without breaking the optimization for other instances, this adds a flag in the appconfig to disable the optimization and actually fetch the correct permissions.

This flag is automatically set by the files_external app when such an external storage is detected

To test:

  • setup an external storage with mountpoint / and set as readonly
  • check if the webui suggests that the home folder is writable

Groupfolders counterpart: nextcloud/groupfolders#4017

@icewind1991icewind1991 added the 3. to review Waiting for reviews label Feb 28, 2025
@icewind1991icewind1991 added this to the Nextcloud 32 milestone Feb 28, 2025
@icewind1991
icewind1991 requested review from a team, skjnldsv and yemkareems and removed request for a teamFebruary 28, 2025 16:49
@icewind1991
icewind1991force-pushed the home-folder-readonly branch 3 times, most recently from 0b62c0e to 1b40100CompareFebruary 28, 2025 17:22
artonge
artonge reviewed Mar 3, 2025
View reviewed changes

@artongeartonge left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Feels like a hack, but I guess there is no better way to do it.
Is that optimization really worth it?

Comment threadapps/files_external/lib/Service/DBConfigService.php Outdated
Comment threadapps/files_external/tests/Service/StoragesServiceTest.php Outdated
Comment threadtests/lib/Files/Node/NodeTest.php Outdated
Comment threadtests/lib/Files/Node/RootTest.php Outdated
@sorbaugh

sorbaugh commented May 12, 2025•
edited
Loading

Copy link
Copy Markdown
Contributor

Fix for #51409

@icewind1991
icewind1991force-pushed the home-folder-readonly branch from 1b40100 to b80b473CompareMay 12, 2025 15:07
@icewind1991
icewind1991 requested a review from a team as a code ownerMay 12, 2025 15:07
@icewind1991

icewind1991 commented May 12, 2025

Copy link
Copy Markdown
MemberAuthor

Feels like a hack

For sure 🙈

but I guess there is no better way to do it.

Not that I can think of

Is that optimization really worth it?

It saves 1 or more cache operation for most requests and is applicable on 99%+ of instances so I would say so

@icewind1991
icewind1991force-pushed the home-folder-readonly branch from b80b473 to f5c5279CompareMay 12, 2025 15:16
@icewind1991
icewind1991 requested a review from artongeMay 14, 2025 14:15
@SebastianKrupinski

SebastianKrupinski commented May 15, 2025

Copy link
Copy Markdown
Contributor

Tested. Works. Properly disables the button when a root folder is read only.

image

@icewind1991
icewind1991force-pushed the home-folder-readonly branch from f5c5279 to b313aaeCompareJune 6, 2025 15:12
come-nc
come-nc requested changes Jun 23, 2025
View reviewed changes
Comment threadapps/files_external/lib/Service/DBConfigService.php Outdated
Comment threadapps/files_external/lib/Service/DBConfigService.php Outdated
Comment threadapps/files_external/lib/Service/StoragesService.php Outdated
Comment threadapps/files_external/tests/Service/StoragesServiceTestCase.php Outdated
Comment threadlib/private/Files/Node/LazyUserFolder.php Outdated
This was referenced Aug 22, 2025
Merged
Merged
Merged
This was referenced Sep 2, 2025
Merged
Merged
@nextcloud-botnextcloud-bot mentioned this pull request Sep 10, 2025
Merged
@nextcloud-botnextcloud-bot mentioned this pull request Sep 18, 2025
Merged
@artonge
artongeforce-pushed the home-folder-readonly branch 2 times, most recently from 0ba0756 to a3f7ed0CompareSeptember 26, 2025 15:31
@artonge
artonge requested a review from a team as a code ownerSeptember 26, 2025 15:31
artonge added a commit to nextcloud/groupfolders that referenced this pull request Sep 26, 2025
@artonge
feat: Add logic to update overwrite homefolder config
3967555
- Related to nextcloud/server#51145
Signed-off-by: Louis Chemineau <louis@chmn.me>
@artonge
artongeforce-pushed the home-folder-readonly branch 2 times, most recently from 3b20fb7 to 079c883CompareSeptember 26, 2025 15:42
@artonge
feat(files): Add appconfig value to disable fixed userfolder permissi…
bd8a0ec
…ons optimization
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Louis Chemineau <louis@chmn.me>
@nextcloud-botnextcloud-bot mentioned this pull request Sep 27, 2025
Merged
@icewind1991

icewind1991 commented Sep 29, 2025

Copy link
Copy Markdown
MemberAuthor

👍

@artonge
feat(files): Mark homefolder as overwritten when an external storage …
553793c
…mounted at / exists
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Louis Chemineau <louis@chmn.me>
@artonge
test(files_external): Ensure Home folder permissions are correct
1ab6dba
Signed-off-by: Louis Chemineau <louis@chmn.me>
@artonge
artonge merged commit f8bed8f into masterOct 1, 2025
225 of 230 checks passed
@artonge
artonge deleted the home-folder-readonly branch October 1, 2025 08:10
@SebastianKrupinskiSebastianKrupinski mentioned this pull request Oct 10, 2025
Closed
8 tasks
@nextcloud-botnextcloud-bot mentioned this pull request Jan 6, 2026
Merged
@icewind1991

icewind1991 commented Jan 14, 2026

Copy link
Copy Markdown
MemberAuthor

/backport to stable32

@backportbotbackportbotBot mentioned this pull request Jan 14, 2026
Merged
2 tasks
@icewind1991

icewind1991 commented Jan 14, 2026

Copy link
Copy Markdown
MemberAuthor

/backport to stable31

@backportbotbackportbotBot mentioned this pull request Jan 14, 2026
Merged
2 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@come-nccome-nccome-nc approved these changes
@SebastianKrupinskiSebastianKrupinskiSebastianKrupinski approved these changes
@skjnldsvskjnldsvAwaiting requested review from skjnldsvskjnldsv is a code owner automatically assigned from nextcloud/server-backend
@yemkareemsyemkareemsAwaiting requested review from yemkareemsyemkareems is a code owner automatically assigned from nextcloud/server-backend
@sorbaughsorbaughAwaiting requested review from sorbaughsorbaugh is a code owner automatically assigned from nextcloud/server-backend
@artongeartongeAwaiting requested review from artongeartonge is a code owner
@provokateurinprovokateurinAwaiting requested review from provokateurin

Assignees

No one assigned

Labels

3. to reviewWaiting for reviews

Projects

None yet

Milestone

Nextcloud 33

Development

Successfully merging this pull request may close these issues.

6 participants

@icewind1991@sorbaugh@SebastianKrupinski@artonge@come-nc@skjnldsv