Fix permissions when copying from ObjectStorage #29115
Conversation
3338b71 to
17b5d9a
Compare
|
@CarlSchwan can you clarify the explanation as it's easy to get confused ? maybe use some example to illustrate what is happening |
|
maybe add some "steps to reproduce" might help to understand better (since we don't have a ticket) |
17b5d9a to
a2a89d6
Compare
I added steps to reproduce section, I hope this makes it more clear |
|
Wouldn't it make more sense to create a new scanner dedicated to object storage? |
There is nothing to scan in a primary object storage, so having one would semantically mismatch. Strange that conceptually it's usually the Scanner that sets the permission. |
I just tested adding the normal Scanner to the ObjectStoreStorage. Since the normal Scanner will still be able to scan one file successfully by using the path. Problem is that this still doesn't solve the problem since the Scanner will ask the Storage for the permissions and the ObjectStoreStorage storage will just returns the permissions from the filecache (this isn't the case for the Local Storage). I used the debugger a bit more and I ended up finding why it put 1 as permission in the filecache. In the ObjectStoreStorage::copyFromStorage when looking for the sourceEntry cache there is a permission mask changing the permissions and these permission are simply copied to the target cache. I pushed a commit that now uses the unjailedStorage to get the sourceEntry, This is that is also done in the Local storage and this seems more correct to me than modifying the Cache class. |
There was a problem hiding this comment.
Instead of unwrapping the Jail wrappers it should unwrap the PermissionsMask
alternatively you can check for $cacheEntry['scan_permissions'], (not available trough a getter on ICacheEntry only trough array access) which is used already used to set the proper permissions while scanner a permissions masked storage
f507a35 to
d26b3d9
Compare
d26b3d9 to
4edbb8a
Compare
80a8c05 to
e1e846f
Compare
|
friendly ping :D |
|
@icewind1991 can you approve ? it follows your suggestion from last time about "scan_permissions" |
|
Ping? |
There was a problem hiding this comment.
Some code comment would be nice as @skjnldsv suggested, otherwise seems fine 👍
Make sure that when a user copy a file from a directory they don't have all permissions to a directory where they have more permissions, the permissions are correctly set to the one from the parent taget folder. This was caused by the ObjectStoreStorage::copyFromStorage using the jailed storage and cache entry instead of the unjailed one like other storages (the local one). Steps to reproduce + Use object storage + Create a groupfolder with one group having full permission and another one who can just read files. + With an user who is in the second group, copy a file from the groupfolder to the home folder of this user. + The file in the home folder of the user will be read only and can't be deleted even though it is in their home folder and they are the owner. In oc_filecache, the permissions stored for this file are 1 (READ) Signed-off-by: Carl Schwan <carl@carlschwan.eu>
e1e846f to
bfa60aa
Compare
|
/backport to stable22 |
|
/backport to stable21 |
Make sure that when a user copy a file from a directory they don't have
all permissions to a directory where they have more permissions, the
permissions are correctly set to the one from the parent taget folder.
This was caused by the ObjectStoreStorage::copyFromStorage using
the jailed storage and cache entry instead of the unjailed one like other
storages (the local one).
Steps to reproduce
who can just read files.
the home folder of this user.
even though it is in their home folder and they are the owner.
In oc_filecache, the permissions stored for this file are 1 (READ)
Signed-off-by: Carl Schwan carl@carlschwan.eu