terms_of_service icon indicating copy to clipboard operation
terms_of_service copied to clipboard

ToS on direct editing

Open LinneyS opened this issue 3 years ago • 5 comments

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?

LinneyS avatar Nov 07 '22 11:11 LinneyS

similar to https://github.com/nextcloud/terms_of_service/pull/765 I guess?

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

nickvergessen avatar Nov 07 '22 11:11 nickvergessen

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

LinneyS avatar Nov 08 '22 07:11 LinneyS

@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?

juliusknorr avatar Nov 08 '22 07:11 juliusknorr

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: https://github.com/nextcloud/terms_of_service/blob/b1f6a4b27096b250848d4b219ee3c856dc7ff78d/lib/Filesystem/Helper.php#L62-L83

nickvergessen avatar Nov 08 '22 07:11 nickvergessen

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

juliusknorr avatar Dec 07 '22 20:12 juliusknorr

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.

max-nextcloud avatar Apr 03 '25 09:04 max-nextcloud

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

nickvergessen avatar Apr 03 '25 09:04 nickvergessen

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.

max-nextcloud avatar Apr 03 '25 11:04 max-nextcloud