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

[Feature Request] Mark mount as read only in the mount settings #29873

Closed
mmattel opened this issue Dec 17, 2017 · 14 comments · Fixed by #36397
Closed

[Feature Request] Mark mount as read only in the mount settings #29873

mmattel opened this issue Dec 17, 2017 · 14 comments · Fixed by #36397

Comments

@mmattel
Copy link
Contributor

mmattel commented Dec 17, 2017

If you mount a storage, it could be that the user has no write rights.
It is not when we do sharing where we can already set a read only flag which we further do/could use (see later).

It targets those mounts where the accessing user with his credentials has no rights to write to. This is handled by the accessed storage and not by owncloud.
I am only talking on full mounts and not about possible different subfolder permissions !

Because we do not see this write disability, a user might try to write into and gets a error message that does not give him the proper cause. If I remember correctly, if we have a shared read only mount, the error message is correct and helpful.

Thinking further, this is also true for the user at the client who has no clue why he can´t write to a mount - and if he does he can do into the client´s filesystem but the data never gets synced.

I do not know if a read only marker for a read only mount gets currently already propagated to the client. If not this should be done and if yes this FR is only an extension if set on the mount. But anyways this is a part of core to do so and a part of the client to take an action which is to be determined.

Proposal:
Adding the ability to mark a storage as read only.

@PVince81 @SamuAlfageme

@ownclouders
Copy link
Contributor

GitMate.io thinks the contributor most likely able to help you is @PVince81.

@PVince81
Copy link
Contributor

Adding a read-only checkbox might actually be a good idea.

The way I see it is that we'd apply the permissions mask storage wrapper on top of that storage to return read-only permissions. So only isReadable() would return true but isCreatable(), and isUpdateable() would return false. I expect the other layers of ownCloud to be able to adjust accordingly and hide write perms everywhere, just like for read-only shares.

@mmattel
Copy link
Contributor Author

mmattel commented Dec 31, 2017

Based on #29999, if implemented:

  1. If a mount is setup, test if you have write access.
  2. If not, propose to set the ready only switch automatically.

@PVince81
Copy link
Contributor

PVince81 commented Jan 9, 2018

Sounds good.

For the technical implementation, whenever the mount has been flagged as "read-only", use the "PermissionsMask" storage wrapper and apply it on this storage.

Some example on how to apply a storage wrapper based on mount config can be found here where we do it for NFD compatibility: https://github.com/owncloud/core/blob/v10.0.3/lib/private/legacy/util.php#L194.

So the tasks would be as follows:

@mmattel
Copy link
Contributor Author

mmattel commented Jan 9, 2018

For your item 3, when I did the test with ftp (see: #29999 (comment)) you can set permissions for files and directories seperated. Knowing this, a test should check both and if one fails, autodetection should go for read only...

@mmattel
Copy link
Contributor Author

mmattel commented May 2, 2018

@PVince81 any update on this?

@mmattel
Copy link
Contributor Author

mmattel commented May 30, 2018

@PVince81 any update on this?

@PVince81
Copy link
Contributor

not scheduled yet

@TJHeeringa
Copy link

@PVince81 any update on this?

@PVince81
Copy link
Contributor

PVince81 commented Sep 10, 2018

This is currently not a priority and we haven't received any contribution with that feature yet.

If anyone feels like implementing it, I've put some hints above.

@TJHeeringa
Copy link

I have completed the first two tasks, but I have not figured out how to write tests yet. I did find some documentation, but hints, like for the second task, would be appreciated.

I want to push my branch, so you can see whether I need to do more, but I get a permission denied. Who do I need to ask to grant me permission to do so?

@phil-davis
Copy link
Contributor

I do not have the jedi powers to grant you branch create access in the owncloud repo. @PVince81 or @DeepDiver1975 can do that.

But you can contribute from your own fork, and make PRs from there - CI will run fine. The process is like:

  • press the "Fork" button on GitHub to make a fork of core repo.
  • clone the forked repo:
git clone https://github.com/my-name/core.git
  • make a "remote" name that points to the original repo, for future convenience:
git remote add upstream https://github.com/owncloud/core.git
git remote -v
  • make your code changes in some branch off master:
git checkout master
git checkout -b stuff
  • make the changes, commit... and git push which will go up to you fork by default
  • go to GitHub and it will suggest making a PR to the original repo

@TJHeeringa TJHeeringa mentioned this issue Oct 5, 2018
11 tasks
@mmattel
Copy link
Contributor Author

mmattel commented Nov 28, 2018

@TJHeeringa may I ping you again ?

@mmattel
Copy link
Contributor Author

mmattel commented Dec 13, 2018

🔉 Ping

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants