Skip to content

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

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

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

Conversation

@icewind1991
Copy link
Member

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

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

@icewind1991 icewind1991 added the 3. to review Waiting for reviews label Feb 28, 2025
@icewind1991 icewind1991 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 team February 28, 2025 16:49
@icewind1991 icewind1991 requested a review from artonge as a code owner February 28, 2025 16:49
@icewind1991 icewind1991 requested a review from sorbaugh February 28, 2025 16:49
@icewind1991 icewind1991 force-pushed the home-folder-readonly branch 3 times, most recently from 0b62c0e to 1b40100 Compare February 28, 2025 17:22
artonge
artonge reviewed Mar 3, 2025
View reviewed changes
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.

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

@sorbaugh
Copy link
Contributor

sorbaugh commented May 12, 2025 •
edited
Loading

Fix for #51409

@icewind1991 icewind1991 force-pushed the home-folder-readonly branch from 1b40100 to b80b473 Compare May 12, 2025 15:07
@icewind1991 icewind1991 requested a review from a team as a code owner May 12, 2025 15:07
@icewind1991
Copy link
Member Author

icewind1991 commented May 12, 2025

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 icewind1991 force-pushed the home-folder-readonly branch from b80b473 to f5c5279 Compare May 12, 2025 15:16
@icewind1991 icewind1991 requested a review from artonge May 14, 2025 14:15
@SebastianKrupinski
Copy link
Contributor

SebastianKrupinski commented May 15, 2025

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

image

@icewind1991 icewind1991 force-pushed the home-folder-readonly branch from f5c5279 to b313aae Compare June 6, 2025 15:12
come-nc
come-nc requested changes Jun 23, 2025
View reviewed changes
This was referenced Aug 22, 2025
Merged
Merged
Merged
This was referenced Sep 2, 2025
Merged
Merged
@nextcloud-bot nextcloud-bot mentioned this pull request Sep 10, 2025
Merged
@nextcloud-bot nextcloud-bot mentioned this pull request Sep 18, 2025
Merged
@artonge artonge force-pushed the home-folder-readonly branch from b313aae to e415894 Compare September 25, 2025 08:37
@artonge artonge force-pushed the home-folder-readonly branch 2 times, most recently from 0ba0756 to a3f7ed0 Compare September 26, 2025 15:31
@artonge artonge requested a review from a team as a code owner September 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 artonge force-pushed the home-folder-readonly branch 2 times, most recently from 3b20fb7 to 079c883 Compare September 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>
@artonge artonge force-pushed the home-folder-readonly branch from 079c883 to 2a7add4 Compare September 26, 2025 15:43
@nextcloud-bot nextcloud-bot mentioned this pull request Sep 27, 2025
Merged
@skjnldsv skjnldsv modified the milestones: Nextcloud 32, Nextcloud 33 Sep 28, 2025
@artonge artonge force-pushed the home-folder-readonly branch from 2a7add4 to a250013 Compare September 29, 2025 09:37
@icewind1991
Copy link
Member Author

icewind1991 commented Sep 29, 2025

👍

@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 force-pushed the home-folder-readonly branch from a250013 to 1ab6dba Compare September 29, 2025 17:26
@artonge artonge merged commit f8bed8f into master Oct 1, 2025
225 of 230 checks passed
@artonge artonge deleted the home-folder-readonly branch October 1, 2025 08:10
@SebastianKrupinski SebastianKrupinski mentioned this pull request Oct 10, 2025
Closed
8 tasks
@nextcloud-bot nextcloud-bot mentioned this pull request Jan 6, 2026
Merged
@icewind1991
Copy link
Member Author

icewind1991 commented Jan 14, 2026

/backport to stable32

@backportbot backportbot bot mentioned this pull request Jan 14, 2026
Merged
2 tasks
@icewind1991
Copy link
Member Author

icewind1991 commented Jan 14, 2026

/backport to stable31

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

Reviewers

@come-nc come-nc come-nc approved these changes

@SebastianKrupinski SebastianKrupinski SebastianKrupinski approved these changes

@skjnldsv skjnldsv Awaiting requested review from skjnldsv skjnldsv is a code owner automatically assigned from nextcloud/server-backend

@yemkareems yemkareems Awaiting requested review from yemkareems yemkareems is a code owner automatically assigned from nextcloud/server-backend

@sorbaugh sorbaugh Awaiting requested review from sorbaugh sorbaugh is a code owner automatically assigned from nextcloud/server-backend

@artonge artonge Awaiting requested review from artonge artonge is a code owner

@provokateurin provokateurin Awaiting requested review from provokateurin

Assignees

No one assigned

Labels

3. to review Waiting 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