Skip to content

fix(collaboration): [OCISDEV-1020] check for proof keys in the templates endpoint - #12852

Open
jvillafanez wants to merge 1 commit into
owncloud:masterfrom
jvillafanez:wopi_template_proof
Open

fix(collaboration): [OCISDEV-1020] check for proof keys in the templates endpoint#12852
jvillafanez wants to merge 1 commit into
owncloud:masterfrom
jvillafanez:wopi_template_proof

Conversation

@jvillafanez

Copy link
Copy Markdown
Member

Description

WOPI templates endpoint will follow the same approach as with the files endpoint, so both will check for proof keys

Related Issue

  • Fixes <issue_link>

Motivation and Context

Allow access only from legit sources.

How Has This Been Tested?

Manually tested with Collabora (with proof keys enabled): it's possible to access without problems.
Trying to access without valid proof keys (or without keys at all) will cause a 500 error, which is what is expected if the verification of the proof keys fails.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

@jvillafanez jvillafanez self-assigned this Aug 26, 2026
@jvillafanez
jvillafanez requested a review from a team as a code owner August 26, 2026 15:58
@update-docs

update-docs Bot commented Aug 26, 2026

Copy link
Copy Markdown

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@kw-security

kw-security commented Aug 26, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@jvillafanez

Copy link
Copy Markdown
Member Author

It seems the fix doesn't work with OnlyOffice because it doesn't send the proof keys to the "/wopi/templates/" endpoint, so the proof verification fails.
There is no problem with Collabora since it sends the proof keys to that endpoint.

@LukasHirt

Copy link
Copy Markdown
Contributor

It seems the fix doesn't work with OnlyOffice because it doesn't send the proof keys to the "/wopi/templates/" endpoint, so the proof verification fails.

So OnlyOffice wouldn't work with oCIS anymore after merging this?

@jvillafanez

Copy link
Copy Markdown
Member Author

So OnlyOffice wouldn't work with oCIS anymore after merging this?

The template feature, which uses the "/wopi/templates" endpoint, would break on OnlyOffice. Regular access using the "/wopi/files" endpoint works fine.
It's possible to disable the proof keys verification, which would fix this problem, but it could open security issues.

@LukasHirt LukasHirt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good. Any follow up on acceptance tests or docs needed?

@LukasHirt

Copy link
Copy Markdown
Contributor

Should we include changelog? Would it also be a breaking change?

@jvillafanez

Copy link
Copy Markdown
Member Author

I'd rather wait for a PM decision.
I've checked with OnlyOffice 9.3, and the acceptance tests use OnlyOffice 9.2.1, so at the moment the PR will break support with the recent versions of OnlyOffice. I don't think it will work on earlier versions neither.

I think the options we have at the moment are:

  • Wait for OnlyOffice to fix the problem. Tests should pass after upgrading the OnlyOffice version without any additional change.
  • Merge the PR as it is. This would break the template feature with OnlyOffice and the only workaround would be to disable the proof key verification, which would put the system at risk. While most of the integration works, having broken features showing errors doesn't look good.
  • Find a different solution. Using the proof keys verification is the simplest option to solve the problem. Other alternatives, if any, will likely require bigger code changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants