fix(headers): only add Cross-Origin-Opener-Policy and Cross-Origin-Embedder-Policy on richdocuments #4207
Conversation
…bedder-Policy on richdocuments Otherwise it's being added to absolutely all requests, creating issues in other apps Closes #4103 Signed-off-by: Thomas Citharel <[email protected]>
tcitworld
requested review from
juliusknorr,
a team and
grnd-alt
and removed request for
a team
| @@ -24,7 +25,7 @@ public function handle(Event $event): void { | |||
| return; | |||
| } | |||
|
|
|||
| if ($this->capabilitiesService->hasWASMSupport()) { | |||
| if ($this->capabilitiesService->hasWASMSupport() && $event->getResponse()->getApp() === Application::APPNAME) { | |||
There was a problem hiding this comment.
As far as I remember this will not work as we require the headers to be set e.g. on the files app as it is the one that iframes Collabora directly.
@caolanm Since you worked on that, can you provide some steps how to test and verify that the wasm support still fully works with that change?
There was a problem hiding this comment.
If not, would it be acceptable to hardcode 'files' (and probably 'files_sharing') as well here?
There was a problem hiding this comment.
Depends, in theory collabora could be loaded by any app, e.g. talk when using the viewer to show a file shared to a conversation.
There was a problem hiding this comment.
Though it is arguable what benefit the wasm experimental feature has there
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
Summary
Otherwise it's being added to absolutely all requests, creating issues in other apps.
However, since I don't know how to test this and why these headers are required, I'm not sure if this still works for public sharing pages for instance.
Ref. #3258 (comment)
TODO
Checklist