Error message hard to understand when sync client is blocked

[schiessle]

As long as the user didn't accepted the ToS, access to the files is blocked by the sync client.

The user gets a sabredav error from the desktop client, something like "file not available". A normal user will never figure out what's the problem and how to solve it. It would be great if we could show a error which tells the user that they have to go to the web interface and accept the ToS to sync their files again.

Maybe the ToS app could through a exception with this error message which than gets forwarded to the clients?

[nickvergessen]

We actually only change the permissions with a storage wrapper. Our API really has some short comings there:

terms_of_service/lib/Filesystem/StorageWrapper.php

Lines 27 to 65 in c479265

	public function isCreatable($path): bool {
	if(!$this->helper->verifyAccess($path)) {
	return false;
	}
	return $this->storage->isCreatable($path);
	}
	public function isUpdatable($path): bool {
	if(!$this->helper->verifyAccess($path)) {
	return false;
	}
	return $this->storage->isUpdatable($path);
	}
	public function isDeletable($path): bool {
	if(!$this->helper->verifyAccess($path)) {
	return false;
	}
	return $this->storage->isDeletable($path);
	}
	public function isReadable($path): bool {
	if(!$this->helper->verifyAccess($path)) {
	return false;
	}
	return $this->storage->isReadable($path);
	}
	public function isSharable($path): bool {
	if(!$this->helper->verifyAccess($path)) {
	return false;
	}
	return $this->storage->isReadable($path);
	}

[tobiasKaminsky]

Maybe we can c&p the mechanism from remote wipe?
There the entire webdav is blocked.

[nickvergessen]

Sorry, but isReadable is overwritten to return false. Should be good enough!?
I also can not see any code that would block to-be-wiped tokens specifically.

[tobiasKaminsky]

Remote wipe is returning 401 UNAUTHORIZED when remote wipe is enabled for that token.

[nickvergessen]

Yeah but we can't/shouldn't do that for ToS, as 401 means throw away your credentials?

[tobiasKaminsky]

I meant this as a way to check how this is done.
For ToS I would use another code, e.g. forbidden?
If then exception class is TOS, client can show correct TOS directly.

[nickvergessen]

Yes, but that is what I tried to say earlier, we only change/overwrite a boolean to false:
#959 (comment)

The wrapper system we have does not allow us passing a reason, app name or anything alike 🙈
And throwing in a boolean check would be breaking behaviour and require fixing many things in many apps that access files

[nickvergessen]

Same is done by access control:
https://github.com/nextcloud/files_accesscontrol/blob/main/lib/StorageWrapper.php#L85

[tobiasKaminsky]

Hm. Maybe a short call?
As I do not want to discuss what is working now, but how we can enhance it :)

[joshtrichards]

Since addressed in various places:

#990
nextcloud/files-clients#64
nextcloud/desktop#7805
nextcloud/desktop#7826