Skip to content
This repository has been archived by the owner on Dec 29, 2022. It is now read-only.

Clients: require source match to differentiate between different iframes of the same origin #47

Merged
merged 1 commit into from
Aug 16, 2018

Conversation

dvoytenko
Copy link
Contributor

The current origin check is necessary, but not sufficient. This PR also adds event.source check on the client side to disambiguate between two iframes with the same origin. With popups such a case is a lot less likely, but for posterity the same check is added for that mode as well.

Copy link

@kaipakartik kaipakartik left a comment

Choose a reason for hiding this comment

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

I am curious as to when this will happen.

When do you have two iframes/popups with the same origin?

origin: 'https://example-sp.com',
source,

Choose a reason for hiding this comment

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

Where is this source variable set?

Shouldn't you have two sources to actually have a test for an iframe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The main source var is set up in the beforeEach of the test. The failure is also tested below - it's the one with the anotherSource var.

@dvoytenko
Copy link
Contributor Author

@kaipakartik Our example is actually slightly hybrid: there are no cases with two iframes running at the same time - instead we destroy the previous iframe slightly later, after the next iframe has been initialized to reduce visual issues. So, they overlap by about 1s. Two iframes running at the same time, however, is not a very bizarre notion - though not the most typical case.

@kaipakartik
Copy link

Thanks for the clarifications.

@dvoytenko dvoytenko merged commit f156c35 into google:master Aug 16, 2018
@dvoytenko dvoytenko deleted the target1 branch August 16, 2018 20:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants