Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(headers): only add Cross-Origin-Opener-Policy and Cross-Origin-Embedder-Policy on richdocuments #4207

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+2 −1
Open

fix(headers): only add Cross-Origin-Opener-Policy and Cross-Origin-Embedder-Policy on richdocuments #4207

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion lib/Listener/BeforeTemplateRenderedListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/
namespace OCA\Richdocuments\Listener;

use OCA\Richdocuments\AppInfo\Application;
use OCA\Richdocuments\Service\CapabilitiesService;
use OCP\AppFramework\Http\Events\BeforeTemplateRenderedEvent;
use OCP\EventDispatcher\Event;
Expand All @@ -24,7 +25,7 @@ public function handle(Event $event): void {
return;
}

if ($this->capabilitiesService->hasWASMSupport()) {
if ($this->capabilitiesService->hasWASMSupport() && $event->getResponse()->getApp() === Application::APPNAME) {
Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

If not, would it be acceptable to hardcode 'files' (and probably 'files_sharing') as well here?

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

Though it is arguable what benefit the wasm experimental feature has there

$event->getResponse()->addHeader('Cross-Origin-Opener-Policy', 'same-origin');
$event->getResponse()->addHeader('Cross-Origin-Embedder-Policy', 'require-corp');
}
Expand Down
Loading