ToS on direct editing

[LinneyS]

Depending on the issue: ONLYOFFICE/onlyoffice-nextcloud#735

May need to allow /directEditing/open request

Hi @juliushaertl . Can you confirm the problem with direct editing?

[nickvergessen]

similar to #765 I guess?

Is an IP list known for OnlyOffice in a similar way?

[LinneyS]

No
ONLYOFFICE does not work by WOPI protocol
Problems with saving with ToS app were not found

Needs a fix to work with direct editing

[juliusknorr]

@nickvergessen What is the general approach of ToS in regards to clients? Maybe we can assume that the ToS are already signed during the account setup for the desktop and mobile clients and could just allow direct editing with the specific app token?

[nickvergessen]

ONLYOFFICE does not work by WOPI protocol

I got that, but basically we need to whitelist the direct editing endpoint like we did the WOPI, but ONLY for requests from the OnlyOffice connection, in case that does not come with user information.

What is the general approach of ToS in regards to clients?

It does not care about the way of interaction. It adds a storage/cache wrapper that removes read, create, update and delete permissions.

Maybe we can assume that the ToS are already signed during the account setup for the desktop and mobile clients and could just allow direct editing with the specific app token?

We exclude skeleton setup, login, login flow and registration:

terms_of_service/lib/Filesystem/Helper.php

Lines 62 to 83 in b1f6a4b

	protected function isCreatingSkeletonFiles(): bool {
	$exception = new \Exception();
	$trace = $exception->getTrace();
	foreach ($trace as $step) {
	if (isset($step['class'], $step['function'])
	&& $step['class'] === 'OC_Util'
	&& $step['function'] === 'copySkeleton') {
	return true;
	}
	if (isset($step['class'])
	&& (
	$step['class'] === LoginController::class
	|| $step['class'] === ClientFlowLoginController::class
	|| $step['class'] === ClientFlowLoginV2Controller::class
	|| $step['class'] === RegisterController::class
	)) {
	return true;
	}
	}
	return false;
	}

[juliusknorr]

I haven't dived into debugging this but looking at the original issue it seems that on the direct editing page the request that javascript performs to the config endpoint fails.

This could be explained as the user is not logged in, so when the file is accessed terms_of_services does not see the already signed terms.

One idea for a fix (but untested) would be to set the current user session to the user id from the direct editing token in
https://github.com/ONLYOFFICE/onlyoffice-nextcloud/blob/504884f16ce2b66e45a66579aa3cdf0fd0197330/controller/editorapicontroller.php#L262 before the file system is setup by getting the file.

You could try doing that through https://github.com/nextcloud/server/blob/215aef3cbdc1963be1bb6bca5218ee0a4b7f1665/lib/public/IUserSession.php#LL64

[max-nextcloud]

I think there were two issues happening here. I think the direct editing one is fixed with #1046. Now the NotPermittedException is thrown once onlyoffice tries to access the document.

Here's the beginning of an exception trace where the onlyoffice apps responds to a apps/onlyoffice/download request with a 403:

{
  "Exception": "OCP\\Files\\NotPermittedException",
  "Trace": [
    {
      "file": "/srv/nextcloud/custom_apps/onlyoffice/lib/Controller/CallbackController.php",
      "line": 336,
      "function": "fopen",
      "class": "OC\\Files\\Node\\File",
      "type": "->"
    },
    {
      "file": "/var/www/html/lib/private/AppFramework/Http/Dispatcher.php",
      "line": 208,
      "function": "download",
      "class": "OCA\\Onlyoffice\\Controller\\CallbackController",
      "type": "->"
    }
  ]
}

So I think what's missing now is what @nickvergessen laid out.

[nickvergessen]

That's not using WOPI anymore?
https://github.com/nextcloud/terms_of_service/blob/master/lib/Checker.php#L137
Then we need to adjust the exclusion pattern

[max-nextcloud]

That's not using WOPI anymore?

It never was. I know it's confusing. Collabora and Officeonline use WOPI, Onlyoffice has it's own protocol.

I prepared a pull request to allow configuring such services on the TOS side of things.