Closed aduffeck closed 8 months ago
(Apologies for the delay, for some reason I did not get a mail notification for this issue)
Interesting scenario indeed, I'd need to check how we react in this case, though I fear we also run in troubles, because user B
eventually has no rights to do anything, and if the application insists in using the WOPI access_token belonging to user B
, any operation fails independently from who holds the lock.
Otherwise, we do have locks associated to the application (and effectively shared among users), but that may not be enough. I'm tempted to say that A
shall not revoke the share until B
is editing the document!
From @micbar:
@DeepDiver1975 @jvillafanez @butonic the wopi docs of ms it clearly states that the locks should not have a user context. The locks are bound to the application. We see a real issue when a user share is revoked during an editing session. The wopiserver should be able to handle that.
We see a real issue when a user share is revoked during an editing session. The wopiserver should be able to handle that.
@micbar for me the issue is not the how - which component should deal with this use case - but the what, i.e. what is the expected behavior. Strictly speaking, if user A
removes write rights to user B
, user B
SHALL NOT be able to keep editing the file. BTW this applies to all applications, not just to WOPI.
On how this is reflected in terms of user experience, I see three options:
1) User B
looses access on the spot, and all uncommitted changes are lost. The application shows permission denied errors.
2) User B
looses access BUT the existing session is "special": B
keeps writing until the editing session is alive, the application just works. If the user disconnects or remains idle for more than 30 minutes, the session expires and a new session cannot be established (permission denied).
3) User A
is told that some user / some application is currently editing the file (as it's locked) and that the removal of permissions has to be postponed until the editing is over.
IMHO 1) is very bad from a UX point of view, 2) is bad from a consistency point of view, as well as from a UX point of view in the case of idle sessions, and 3) is (barely) viable. If you see more options, they're welcome!
@glpatcern All 3 cases are difficult, I agree.
I was not referring to the actual write operation itself, i was referring to the lock/unlock.
In all 3 cases, we currently have a lock on the file which can not be unlocked by the wopiserver. This causes the file to be locked until the TTL is expiring.
My point was, that in all 3 cases, independently from the writing of the file, the file should be unlocked.
My point was, that in all 3 cases, independently from the writing of the file, the file should be unlocked.
I see now, and that's right. I should check what happens in our implementation then, where locks are not per-user but per-app (and the unlock should just work). But in a way that's only the tip of the iceberg...
OK, the result of my test is that we implement option 1:
AND as the lock is shared, the owner (or whoever else with appropriate rights) can take it over and keep editing:
Behind the scene, the lock was refreshed successfully: in eos the lock may belong to the app or to the user, and for this case we make the lock owned by the app only.
So, the issue with the unlock / refreshLock operations not being honored is actually a storage implementation issue, not a wopiserver issue. But the general picture is that there's room for improvement.
We had a quick discussion internally, and as a matter of fact the other options (2 and in particular 3) are far from viable: it's essentially impossible to know in how many ways (via sync, apps, web uploads, FUSE access, ...) multiple user B
s are interacting with the storage on a share with potentially many folders and files, and user A
can't really be preempted in revoking a share. Therefore option 1 (the current implementation in Reva) holds.
Now the fact that Reva edge binds the lock with the user in such a way that Unlock or RefreshLock are not possible, that's a Reva edge issue, not a wopiserver issue, as with Reva master we are clear. Therefore this issue can be closed.
We ran into a problem with wopi locks when grants are being revoked. The scenario is as follows:
A
creates a file and shares it with userB
B
opens the file in the application which creates a WOPI lock on the fileA
revokes the according grant while the lock is still in placeAs a result
B
can no longer get the lock and unlock the file but trying to do so triggers the application to try and create a new lock next time userA
opens the file, which doesn't work because the old lock is still in place and doesn't match.The root problem seems to be that the locks are being held by the users, not the wopiserver/application. We were pondering about a way to prevent this issue but could only come up with the idea to use service accounts (with permissions to get, set and delete locks for all files) for the lock operations, effectively scoping the locks to the application instead of the user.
Do you think that's the way to go about this or do you maybe have another idea?
/cc @butonic @2403905
P.S.: The service account would maybe also need stat permsisions because the current code sometimes stats before working with the lock. We aren't sure whether that's actually needed however, but that's a different topic and not really relevant to the architectural decision.