Add COOP+COEP+CORP headers #6597

somebody1234 · 2023-05-08T06:19:23Z

Pull Request Description

Adds COOP+COEP+CORP headers. This is mainly required for high resolution timing (on Chrome, it increases performance.now resolution from 100us to 5us)

Important Notes

None

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

The documentation has been updated, if necessary.
Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
All code follows the
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
All code has been tested:
- Unit tests have been written where possible.
- If GUI codebase was changed, the GUI was tested when built using ./run ide build.

somebody1234 · 2023-05-08T06:50:37Z

mini self QA:
✔️ self.crossOriginIsolated === true + 5ms resolution performance.now on npm run watch-dashboard
✔️ self.crossOriginIsolated === true + 5ms resolution performance.now on ./run gui watch
✔️ self.crossOriginIsolated === true + 5ms resolution performance.now on ./run ide watch
✔️ self.crossOriginIsolated === true + 5ms resolution performance.now on ./run ide build
✔️ google login still works on npm run watch-dashboard
❓ oauth still works on desktop IDE - i cannot test as i'm not on macos

issues, unrelated to this PR:
❌ github login works on npm run watch-dashboard - it fails with OAuth - Error handling auth response. Error: User+is+not+confirmed.+ at @aws-amplify/auth/src/OAuth/OAuth.ts:222 (handleAuthResponse)
❌ oauth works on gui watch - this is a known issue. the main issue is that localhost:8080 isn't a valid redirect url for cognito

wdanilo · 2023-05-10T04:37:07Z

app/ide-desktop/lib/client/src/bin/server.ts

+            response.setHeader('Cross-Origin-Embedder-Policy', 'require-corp')
+            response.setHeader('Cross-Origin-Opener-Policy', 'same-origin')


Please add description + links why we need it

wdanilo · 2023-05-10T04:37:41Z

app/ide-desktop/lib/content/src/serviceWorker.ts

@@ -0,0 +1,32 @@
+/** @file A service worker that redirects paths without extensions to `/index.html`.
+ * This is only used in the cloud frontend. */


What is "cloud frontend"? I did not hear this name earlier.

wdanilo · 2023-05-10T04:37:51Z

app/ide-desktop/lib/content/src/serviceWorker.ts

@@ -0,0 +1,32 @@
+/** @file A service worker that redirects paths without extensions to `/index.html`.
+ * This is only used in the cloud frontend. */
+/// <reference lib="WebWorker" />


What is this comment about?

wdanilo · 2023-05-10T04:37:56Z

app/ide-desktop/lib/content/src/serviceWorker.ts

+        event.respondWith(
+            fetch(event.request.url).then(response => {
+                const clonedResponse = new Response(response.body, response)
+                clonedResponse.headers.set('Cross-Origin-Embedder-Policy', 'require-corp')


Please add description + links why we need it

wdanilo · 2023-05-10T04:39:21Z

app/ide-desktop/lib/content/src/serviceWorker.ts

+                clonedResponse.headers.set('Cross-Origin-Opener-Policy', 'same-origin')
+                clonedResponse.headers.set('Cross-Origin-Resource-Policy', 'same-origin')
+                return clonedResponse
+            })
+        )


Also:

this is the same as code in other place - can we refactor it to one place maybe?

We have server built-into electron, why are we not using the same server with the watch scripts? would it not be cool to use the same code + make it possible to spawn electron in the watch mode? Ofc this is not part of this task, just a more general question.

./run ide watch does use the same server, i believe, but ./run gui watch doesn't spawn electron at all. it might be possible to spawn electron on ./run gui watch, but in that case i'm not sure whether that would do anything different to ./run ide watch?

wdanilo · 2023-05-10T04:39:56Z

app/ide-desktop/lib/dashboard/src/serviceWorker.ts

+            ? fetch('/index.html')
+            : fetch(event.request.url)
+        event.respondWith(
+            responsePromise.then(response => {
+                const clonedResponse = new Response(response.body, response)
+                clonedResponse.headers.set('Cross-Origin-Embedder-Policy', 'require-corp')
+                clonedResponse.headers.set('Cross-Origin-Opener-Policy', 'same-origin')
+                clonedResponse.headers.set('Cross-Origin-Resource-Policy', 'same-origin')


This is 3rd place with the same code.

wdanilo · 2023-05-10T11:45:39Z

I tested it and it does seem to not break anything. I did not test loging with the new dashboard as its not enabled and I don't know how to run it, but if it did not break login using Google, then it should not influence anything else.

* develop: Limit the number of reported warnings (#6577) Add COOP+COEP+CORP headers (#6597) Fix issues with missing sourcemaps (#6572) Fix asset delete; implement project delete and project rename (#6566) Fix #6377: Change ctrl-r shortcut (#6620)

This reverts commit 9597dd3.

…ing-6287 * develop: Fix shortcuts table formatting (#6644) Automatic type based dropdown does not include singleton in a union type (#6629) Make Meta.get_annotation work for constructor (#6633) Limit the number of reported warnings (#6577) Add COOP+COEP+CORP headers (#6597)

This PR reverts latest changes regarding introducing COOP COEP CORP headers. It broke the map visualization (#6645)

This reverts commit d853851.

* develop: Implement loading spinner for visualisations. (#6512) Fix blank input port (#6614) Add `Date_Range` (#6621) All Vector operations shall be applicable on java.util.ArrayList (#6642) Fix redirect paths and enable authentication and new dashboard by default (#6605) Fix #6287: wrong nested breadcrumb ordering (#6617) Whitelist AWS Cognito domains (#6643) Revert "Add COOP+COEP+CORP headers (#6597)" (#6647) Fix shortcuts table formatting (#6644) Automatic type based dropdown does not include singleton in a union type (#6629) Make Meta.get_annotation work for constructor (#6633)

* Revert "Revert "Add COOP+COEP+CORP headers (#6597)" (#6647)" This reverts commit d853851. * Fix loading third-party resources

Add COOP+COEP+CORP headers

a4e3843

somebody1234 requested review from PabloBuchu and indiv0 as code owners May 8, 2023 06:19

somebody1234 requested a review from wdanilo May 8, 2023 06:19

somebody1234 added the CI: No changelog needed Do not require a changelog entry for this PR. label May 8, 2023

wdanilo requested changes May 10, 2023

View reviewed changes

somebody1234 added 2 commits May 10, 2023 16:29

Address review

d1bf424

Address review

e05cdb4

Merge branch 'develop' into wip/sb/coop-coep-corp

78e251c

wdanilo merged commit 9597dd3 into develop May 10, 2023

wdanilo deleted the wip/sb/coop-coep-corp branch May 10, 2023 11:47

PabloBuchu added a commit that referenced this pull request May 11, 2023

Revert "Add COOP+COEP+CORP headers (#6597)"

c40a4b5

This reverts commit 9597dd3.

mergify bot pushed a commit that referenced this pull request May 11, 2023

Revert "Add COOP+COEP+CORP headers (#6597)" (#6647)

d853851

This PR reverts latest changes regarding introducing COOP COEP CORP headers. It broke the map visualization (#6645)

somebody1234 added a commit that referenced this pull request May 11, 2023

Revert "Revert "Add COOP+COEP+CORP headers (#6597)" (#6647)"

8657fe3

This reverts commit d853851.

somebody1234 mentioned this pull request May 11, 2023

Add COOP+COEP+CORP headers #6646

Merged

5 tasks

PabloBuchu pushed a commit that referenced this pull request May 12, 2023

Add COOP+COEP+CORP headers (#6646)

4b64d66

* Revert "Revert "Add COOP+COEP+CORP headers (#6597)" (#6647)" This reverts commit d853851. * Fix loading third-party resources

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add COOP+COEP+CORP headers #6597

Add COOP+COEP+CORP headers #6597

somebody1234 commented May 8, 2023

somebody1234 commented May 8, 2023

wdanilo May 10, 2023

wdanilo May 10, 2023

wdanilo May 10, 2023

wdanilo May 10, 2023

wdanilo May 10, 2023

somebody1234 May 10, 2023

wdanilo May 10, 2023

wdanilo commented May 10, 2023

		response.setHeader('Cross-Origin-Embedder-Policy', 'require-corp')
		response.setHeader('Cross-Origin-Opener-Policy', 'same-origin')

		@@ -0,0 +1,32 @@
		/** @file A service worker that redirects paths without extensions to `/index.html`.
		* This is only used in the cloud frontend. */

Add COOP+COEP+CORP headers #6597

Add COOP+COEP+CORP headers #6597

Conversation

somebody1234 commented May 8, 2023

Pull Request Description

Important Notes

Checklist

somebody1234 commented May 8, 2023

wdanilo May 10, 2023

Choose a reason for hiding this comment

wdanilo May 10, 2023

Choose a reason for hiding this comment

wdanilo May 10, 2023

Choose a reason for hiding this comment

wdanilo May 10, 2023

Choose a reason for hiding this comment

wdanilo May 10, 2023

Choose a reason for hiding this comment

somebody1234 May 10, 2023

Choose a reason for hiding this comment

wdanilo May 10, 2023

Choose a reason for hiding this comment

wdanilo commented May 10, 2023