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

Expose origin attribute in PortalActivateEvent #162

Open
shhnjk opened this issue Nov 22, 2019 · 12 comments
Open

Expose origin attribute in PortalActivateEvent #162

shhnjk opened this issue Nov 22, 2019 · 12 comments
Labels
design work needed An acknowledged gap in the proposal that needs design work to resolve

Comments

@shhnjk
Copy link

shhnjk commented Nov 22, 2019

I can imagine a future where a site is expecting a data in PortalActivateEvent from trusted site.
But PortalActivateEvent can be triggered by any site (similar to message event), therefore origin attribute has to be exposed in PortalActivateEvent. Otherwise it'll be an XSS nightmare 😊

@jeremyroman
Copy link
Collaborator

Interesting suggestion. I think that the existing API surface is sufficient to allow authors to figure out what's going on here, but plumbing the origin here seems like it would be reasonably straightforward and be much easier to use for authors.

@a4sriniv, you looked at messaging APIs; WDYT of this?

@a4sriniv
Copy link
Contributor

I think this is a good idea. I think of the data field in the portalactivate event as an author friendly alternative to just using postMessage before activation, so it makes sense to me to mirror the origin aspect of the message event (and it shouldn't be difficult to implement).

But on a related note, we also have targetOrigin specified as part of the postMessage API parameters, which checks the origin of the destination frame before delivering the message. Do we need something similar for activate?

@jeremyroman
Copy link
Collaborator

But on a related note, we also have targetOrigin specified as part of the postMessage API parameters, which checks the origin of the destination frame before delivering the message. Do we need something similar for activate?

An interesting thought. What would the semantics of that be if the guest contents navigates? Also, should it only limit delivery of the activation data, or should it block activation altogether?

@shhnjk
Copy link
Author

shhnjk commented Nov 22, 2019

targetOrigin Is also good to add :)

@a4sriniv
Copy link
Contributor

An interesting thought. What would the semantics of that be if the guest contents navigates? Also, should it only limit delivery of the activation data, or should it block activation altogether?

My initial thought here was to fail the activation (resolve the promise with an error) as a guard against the guest contents navigating without the host's knowledge.

@domenic
Copy link
Collaborator

domenic commented May 8, 2020

Is this still a good idea, given our desire to limit cross-origin communication?

@domenic domenic added the spec todo A nitty-gritty detail that needs spec work label May 8, 2020
@a4sriniv
Copy link
Contributor

Yeah I don't think this applies anymore. I think we're going to omit portalactivatedata from the activate event for cross-origin portal activations.

@domenic
Copy link
Collaborator

domenic commented May 11, 2020

Sounds good. Let's close this for now. I filed #196 to track speccing those restrictions.

@domenic domenic closed this as completed May 11, 2020
@jakearchibald
Copy link
Collaborator

Are we dropping adoption in the activate event? Otherwise, it seems like the page should know the origin of the page it's adopting.

@jeremyroman jeremyroman reopened this May 13, 2020
@a4sriniv
Copy link
Contributor

Ahh, I remembered this issue as using targetOrigin to limit which origins get activate data, I forgot about actually using the targetOrigin to stop activation altogether.

@domenic
Copy link
Collaborator

domenic commented May 20, 2020

I'm a bit confused where this landed. I think there are several proposals being floated:

  • Add a targetOrigin parameter to activate(), such that if the portal is currently navigated to something that doesn't match targetOrigin, then activate() fails.
  • Add an origin member to PortalActivateEvent. This lets the portaled page know who is activating them. For example, it could let them better decide whether to call adoptPredecessor(), or how to interpret the data in event.data (e.g. "if activated by Aggregator X, then the data will probably be in Aggregator X format").
    • Since we are banning sending data in cross-origin activation, and potentially dropping adoptPredecessor(), this seems less useful. But I could still imagine use cases, for example displaying a "back to aggregator" link.
    • And, I just like the symmetry with MessageEvent.
    • But perhaps this is a cross-origin data leak?

Are either of these good ideas? Both of them?

@domenic domenic added design work needed An acknowledged gap in the proposal that needs design work to resolve and removed spec todo A nitty-gritty detail that needs spec work labels May 20, 2020
domenic added a commit that referenced this issue May 20, 2020
@jeremyroman
Copy link
Collaborator

We could defer adding this to later if we wanted to, though I suspect it will eventually be useful especially in combination with adoptPredecessor.

There might be a world in which we become worried about sites setting using their hostname to convey information (though I'm not convinced that is convenient enough that it would actually be in widespread use), but that will arise elsewhere in the platform (location.ancestorOrigins etc) and mitigations exist (like heuristically or unconditionally censoring to eTLD+1).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design work needed An acknowledged gap in the proposal that needs design work to resolve
Projects
None yet
Development

No branches or pull requests

5 participants