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.

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

Add docs for HTTP Permissions-Policy header cross-origin-isolated directive #36780

Merged
merged 18 commits into from
Dec 22, 2024

Conversation

@github-actions github-actions bot added Content:WebAPI Web API docs Content:HTTP HTTP docs size/m [PR only] 51-500 LoC changed labels Nov 13, 2024
@skyclouds2001 skyclouds2001 marked this pull request as ready for review November 13, 2024 20:53
@skyclouds2001 skyclouds2001 requested review from a team as code owners November 13, 2024 20:53
@skyclouds2001 skyclouds2001 requested review from sideshowbarker and hamishwillee and removed request for a team November 13, 2024 20:53
@sideshowbarker sideshowbarker removed their request for review November 14, 2024 09:02
@skyclouds2001 skyclouds2001 requested a review from a team as a code owner November 18, 2024 18:28
@skyclouds2001 skyclouds2001 requested review from Josh-Cena and removed request for a team November 18, 2024 18:28
@github-actions github-actions bot added the Content:JS JavaScript docs label Nov 18, 2024
@Josh-Cena Josh-Cena removed their request for review December 15, 2024 01:28
Comment on lines 108 to 109
- {{HTTPHeader("Cross-Origin-Opener-Policy")}} with `same-origin` as value (protects your origin from attackers)
- {{HTTPHeader("Cross-Origin-Embedder-Policy")}} with `require-corp` or `credentialless` as value (protects victims from your origin)
Copy link
Collaborator

@hamishwillee hamishwillee Dec 16, 2024

Choose a reason for hiding this comment

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

The "(protects your origin from attackers)" and same on the other header are wrong/pointless. I will fix.

Copy link
Member

Choose a reason for hiding this comment

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

The explanations here makes sense to me though. With COOP your site can't be cross-origin accessed by another opener. With COEP you can't cross-origin access another site.

Copy link
Collaborator

@hamishwillee hamishwillee Dec 16, 2024

Choose a reason for hiding this comment

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

OK, so Cross-Origin-Opener-Policy certainly protects your origin from attackers, since it allows you to prevent your document from loading cross origin resources in the same BCG or being loaded into same BCG as cross origin documents, and severs the relationships.

The embedder policy ensures that your document only loads those documents that have opted into CORS, or doesn't send them credentials. I guess that you could look at that as "protect victims from your origin"? I'm not actually entirely sure why that has value.

I am sure of two things:

  1. If you don't know about these before, having the text here won't help you.
  2. None of this should be here IMO. I have moved the relevant docs into the cross origin isolation doc and cross linked.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Upshot, love an explanation of why COEP is useful other than you now need it to access certain APIs. But either way, it won't change this doc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

a useful link for Cross-Origin isolation by stackblitz - https://blog.stackblitz.com/posts/cross-browser-with-coop-coep/

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, that is cool. A little out of date but well worth linking.

@hamishwillee
Copy link
Collaborator

@skyclouds2001 I have rewritten this to my taste, mostly to remove some of the duplication around these headers into the self.crossOriginIsolation docs. Also though to make it more clear that crossOriginIsolated has a bunch of meanings - the main one here is "access to the API", but there are others, such as "severing of opener relationships".

There is quite a lot of change so now I can't be objective. Can we swap roles now so you review my changes and make sure they largely make sense?

@skyclouds2001
Copy link
Contributor Author

#37287 fired for a feature request for a new glossary for Cross-Origin isolation

@hamishwillee
Copy link
Collaborator

#37287 fired for a feature request for a new glossary for Cross-Origin isolation

I'm not sure whether or not this is a good idea. I actually tried it when I made the last round of edits. The question was really "what should we include". If you include all the information in the window.crossOriginIsolated property then you have to duplicate most of it anyway for this topic - which defeats the purpose. If you don't include it, then it doesn't really help having an extra topic.

So "maybe". Wouldn't block this on that.

@skyclouds2001 Have you had a chance to review my changes? I'm shortly on holiday and I don't want to leave this dangling if we can avoid it.

@skyclouds2001
Copy link
Contributor Author

@skyclouds2001 Have you had a chance to review my changes? I'm shortly on holiday and I don't want to leave this dangling if we can avoid it.

Sure!

@skyclouds2001
Copy link
Contributor Author

I'm not sure whether or not this is a good idea. I actually tried it when I made the last round of edits. The question was really "what should we include". If you include all the information in the window.crossOriginIsolated property then you have to duplicate most of it anyway for this topic - which defeats the purpose. If you don't include it, then it doesn't really help having an extra topic.

I think the glossary could include details about the cross-origin isolated, most content could be extract from the current docs for Window.crossOriginIsolated and WorkerGlobalScope.crossOriginIsolated.

@hamishwillee
Copy link
Collaborator

Let me know when you have finished your review.

We could try do the glossary in #37287 now, or hold off. I'm tempted to merge this and then do the glossary.

@hamishwillee
Copy link
Collaborator

I think the glossary could include details about the cross-origin isolated, most content could be extract from the current docs for Window.crossOriginIsolated and WorkerGlobalScope.crossOriginIsolated.

Worth a try, but do we do it now, or after merging this?

@skyclouds2001
Copy link
Contributor Author

skyclouds2001 commented Dec 20, 2024

I think it is better to do after merge this first, or the change will be too huge.

```

To check if cross origin isolation has been successful, you can test against the {{domxref("Window.crossOriginIsolated")}} property or the {{domxref("WorkerGlobalScope.crossOriginIsolated")}} property available to window and worker contexts:
To use shared memory your document must be in a [secure context](/en-US/docs/Web/Security/Secure_Contexts) and {{domxref("Window.crossOriginIsolated","cross-origin isolated","","nocode")}}.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
To use shared memory your document must be in a [secure context](/en-US/docs/Web/Security/Secure_Contexts) and {{domxref("Window.crossOriginIsolated","cross-origin isolated","","nocode")}}.
To use shared memory, your document must be in a [secure context](/en-US/docs/Web/Security/Secure_Contexts) and {{domxref("Window.crossOriginIsolated", "cross-origin isolated", "", 1)}}.

Copy link
Collaborator

Choose a reason for hiding this comment

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

While the only requirement is that the value of the parameter is set, "nocode" is the defacto standard on MDN. The reason being that it is obvious to authors reading the source what the intent is - so it is better.

@hamishwillee
Copy link
Collaborator

I think it is better to do after merge this first, or the change will be too huge.

And there is some disagreement on the glossary issue about how to approach it.

Copy link
Collaborator

@hamishwillee hamishwillee left a comment

Choose a reason for hiding this comment

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

Thanks for all your work on this @skyclouds2001 - and for the follow on review. Merging now.

@hamishwillee hamishwillee merged commit 6d6c727 into mdn:main Dec 22, 2024
8 checks passed
@skyclouds2001 skyclouds2001 deleted the cross-origin-isolated branch December 23, 2024 03:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:HTTP HTTP docs Content:JS JavaScript docs Content:WebAPI Web API docs size/m [PR only] 51-500 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants