Wrong argument order in a call to `generateFileToken`

[mikekaganski]

https://github.com/nextcloud/richdocuments/blob/master/lib/TokenManager.php#L286 calls generateFileToken, and passes $direct where generateFileToken expects $hideDownload.

See discussion at https://github.com/nextcloud/richdocuments/commit/28c0e502ba05f125cdf955de2658278ac07d751f#.

[gitsan13]

Hi! Will you accept a PR for this? I would like to look into it:)

[Raudius]

Hi @gitsan13

Contributions are very welcome :)

[gitsan13]

Thank you for the approval @Raudius :)

[gitsan13]

As I looked into the file TokenManager.php, this is the anamoly in the generateToken function that I found,

[gitsan13]

Could you please throw some light on what is the 'shareToken' value that is passed in both the cases? Also, what would be the 'templateDestination' and 'hideDownload' field values if the capabilitiesService object has no template source?

[Raudius]

@gitsan13

Thanks for looking into this issue.

Could you please throw some light on what is the 'shareToken' value that is passed in both the cases?

The shareToken is an identifier for a "share" object. This argument is only required when we are opening a file that has been shared. In this case we are creating a new file so a shareToken would not be expected.

Also, what would be the 'templateDestination' and 'hideDownload' field values if the capabilitiesService object has no template source?

The template destination I think can be kept as $targetFile->getId().

The hideDownload field should be false. This is only true for shared files that have the download permission removed.

[gitsan13]

Heyy thanks for explaining it so clearly :) Here are the changes that I made to the generateFileToken function:-

Does the changes look apt to you?

[Raudius]

I think the changes are not quite right. But I could not find much documentation online, so here is what I have gathered on what the "has template source" check is about.

• Newer versions of Collabora allow servers to define a resource where document templates are stored. This is passed during the CheckFileInfo request https://github.com/nextcloud/richdocuments/blob/master/lib/Controller/WopiController.php#L230
• If the Collabora server accepts this resource we can then create a new file from a template by just passing the template's ID when creating the file.
• If the Collabora server does not accept the template source, then we need to use the "legacy" method. This involves passing the template file to collabora directly, and then defining the "template destination" as our new file.
• The legacy version is a bit hacky as we have to juggle references to two different files for the WOPI token depending on the request to make sure the file is created with the template (but we save the changes to the destination file). With the new method Collabora takes care of creating the file from the template so we can just keep the reference to the file (not the template). It is just kept for legacy reasons (so older versions of Collabora still work).

In summary I think the legacy function does not need many changes, the only issue currently is that it is missing the $allowDownload parameter, before the $direct parameter.

Also I think we can continue to omit the $share parameter, because the default (null) value is sufficient.

[gitsan13]

Hi @Raudius I went through your concise explanation and came up with the following changes:-

As you said, if the Collabora server does not accept the request by CheckFileInfo() method to define the resource for storing templates, then we need to pass the legacy method. In the legacy method, we need to add the allowDownload parameter to make necessary modification. In the image attached below, I thought of using "DownloadAsPostMessage" to get Download option in the legacy method.

The final changes made to the TokenManager.php file is attached below:-

What do you say?

[joshtrichards]

Fixed / no longer applicable.