-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Sharing a file does set while to rw for short time #11707
Comments
What currently happens is:
We have to make sure that step 4 either happens before step 3 or set non destructive permissions per default. |
Then the file should maybe only be shared once the popup is closed? |
That would be another approach that I'd welcome. @PVince81 Is that easily possible? |
But that's a behaviour change and it might not be obvious that people have to close it ("i clicked share but my friend didnt get the mail"). I vote for having non destructive permissions as default. |
I agree with that, but having read-only as default is an even bigger change in behavior. Sharing with someone on your same instance should by default be read/write of course. |
The other alternative would be to only add the share after the dropdown is closed (or the window is closed). Basically: defer changes made in the dropdown until it's closed. |
@PVince81 that’s what I said above. ;) |
Ah right, missed that. |
Not sure If I want to wait until the drop down was closed before the shares are stored for various reasons:
If we agree that we need to do something I would vote for minimal permissions by default. But honestly I'm not sure if we really need to do something here. We are talking about seconds, right? How likely is it that at the time I shared a file with a user, the user is logged in, refresh his browser directly after I clicked the share button, realize the new share within the 2-3 seconds time frame and clicks on edit, edits the file and save it again? Is this really a problem? |
Well, this is something that you can easily automate and therefore: yes it is. There is a reason that no other platform implements this in the way we do :-) |
Well, but then it is clearly a destructive behaviour and nothing which happens by accident. Also we are talking about internal shares with a known user with whom I actually want to share stuff, not about a random person getting access and go crazy. Let's say someone in your company does this, running a script to destroy all documents shared with him. In this case you would handle this person on a personal level anyway. The same way he could run in your office and run all your paper documents through a document shredder. 😉 |
Well, there are actually use-cases where you want to prevent such destructive behaviour ;-) Easiest example: Teacher wants to share documents with the scores with the students, in a short moment they are able to modify them. We either have to:
|
Then I would go with 2. Everything else is imho to much extra complexity with respect to the problem we want to solve. |
I agree. |
@LukasReschke @jancborchardt @schiesbn @PVince81 So the default permission should be read-only? @karlitschek Is this still topic of 8.0? Otherwise move this to the 8.1 milestone and I will have a look later on this. |
Yes. I'd vouch for 8.1. Nothing did explode in the past due to that and as we're already in the beta let's keep it that way. |
I’d vouch for 8.1 if anything as well.
I’m really not too fond about that. We didn’t have real problems in practice before because of this. And I think we would get more complaints about people not noticing sharing is read-only by default and others can’t edit it. |
Well - fact is that we do it differently than everybody else and this really a problem in serious environments. You simply don't want to have it that way. So we either set it to read-only or we come up with a proper sharing popup dialog (as does Dropbox or Google Drive) but the current solution is a mess at best. |
The current approach can be kinda compared to creating a user and give it admin rights per default and then have a small timeframe where the user has administrative privileges. A best practise is to obey the least-privilege principle: Give the user only what he needs at start and if he needs more give him more permissions. Honestly, if you want to have a file editable we have the checkbox right there in the sharing dropdown and doing it this way would not result in any potentially unintentional behaviour. Then again we also have public shared files only readable by default. For the regular user it is in no way understandable why sharing a file with other users should make it editable but sharing a file publicly will make it read-only. |
Hmm. I see pros and cons for both options. I would leave it as it is for now because we are in feature freeze and no one complained about the current behavior -> 8.1 |
Sane defaults are the defaults that administrator set for this object type. Administrator. Not developer. What users may or may not forget, is not yours to call. You can't hold everyone's hand in the world. There's roughly 3 billion internet users in the world, and you only have two hands. If you are going to intentionally create administration headache, you are limiting the potential of your product, and encouraging its users to look for alternatives. |
Most important is to cover the most likely scenario with the defaults. Resharing should be off by default. That should be an act of commission - you want someone to get a copy of a copy, therefore you turn on resharing. Otherwise, it should be off. Editing should be on, that is why you share with someone more often than not. Delete should be off inside edit, so we don't have people losing data by accident. That covers the most likely path for most users, and then the others can be configured at the time of share. |
@MTRichards , I think we're leaning to discuss apples and oranges. |
Sure, I understand - I needed to clarify the defaults, which I did. As for the issue of defaults, and being able to change them in a config switch or something, this is tricky. Every time we add a config file option, we increase the test matrix, complicate the code, and make it harder to maintain. We also increase confusion, the number of support cases, and make it harder for an admin to easily get what they want out of ownCloud. Eventually we will get bogged down and unusable. We have a TON of these sorts of cases out there, just look at the sample config file - it is daunting and awesome at the same time. So the question becomes balance - to balance the needs of most against the needs of few and come up with the right answer. In this situation, if a read only folder is needed more than a read/write folder - I am just not yet convinced that there are enough use cases or need to warrant a switch. Are there use cases? You bet. Are there enough to warrant the addition of these switches? Not convinced. |
@AnrDaemon There might be a way for you to override this default with a custom theme or app. But unfortunately we have to limit the number of configuration options as @MTRichards said. In a lot of cases we have to look at what 95% or all users want and what 95% or all other sync and share solutions out there do. And this is RW. We really understand the desire and need for an non standard behavior. |
So it seems we are still devided what the default behaviour has to be. As I think we have seen all possible combinations of. READ/WRITE/SHARE come along here. And for each combination there are very valid use cases. If we would eventually move to 'simpler' permissions (#12970) so only can edit and can share. Maybe the approach from @LukasReschke in #11707 (comment) is best. So next to the input field for the sharee you have a dropdown menu where you can select the behaviour your want. |
I actually think that the current behavior is fine because it is the most common behavior and it also the same that all our competitors do. Which means that this is also what users expect. |
It's not really, see #11707 (comment) |
All others: Set to "edit" per default BUT allow changing this BEFORE sharing. Thus not setting potential insecure defaults. The screenshots in #11707 (comment) show this pretty well. It is possible to set the permissions before anything is shared, and this is the default sharing screen. Let's set this to backlog… |
I see two possible solutions:
Hope this clarifies that it is not the defaults that are wrong, but the moment we create the share. |
The security issue is this one: imagine a funny student who configured his client to automatically delete all new share. The proffessor want to share a folder containing the assignement data and instruction with all the students (the students group) When he does so, he better be quick to remove the write permission before it's too late and all the data is gone. |
🙈 That's a weird scenario. |
Weird, may be. |
Well – this should simply result in that person unsharing the share from themselves, right? Not in deleting all the files. |
Not if the student is fast enough with deleting before the professor sets R-only. I still think it is a security issue. |
👍 |
It is now possible to set default sharing permissions since OC 10.0.3, closing. |
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. |
When sharing a file via the sharing button to other users the file is automatically shared with "can edit" permissions. - While permissions can be changed then in the next window within a short timeframe the receiving user has permissions to edit the shared file which is an unexpected behaviour.
We should make sure that always the least permissive permissions are set per default and/or make this editable within the same step.
@jancborchardt Your opinion?
\cc @guruz @ogoffart
The text was updated successfully, but these errors were encountered: