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

false childOrigin to skip origin check #74

Merged
merged 7 commits into from
Oct 14, 2021

Conversation

KutnerUri
Copy link
Contributor

@KutnerUri KutnerUri commented Oct 12, 2021

Allow consumers to skip iframe origin url check in connectToChild().
This is useful when the iframe redirects, etc.

connectToChild() already checks the message origin is the target iframe, so it should be ok.

Closes #73

@KutnerUri KutnerUri mentioned this pull request Oct 12, 2021
@KutnerUri
Copy link
Contributor Author

I added the same logic for originForReceiving. not sure it makes sense, but the tests pass

@Aaronius
Copy link
Owner

Thanks for the contribution, @KutnerUri! I left a comment to consider on the issue you logged.

I think I'd like to support a childOrigin value of * rather false, as it's more consistent the wildcard pattern provided underneath the hood by postMessage and also cuts down on some code. I've added commits accordingly. I'd appreciate it if you would review my changes and then I'd like to take another look myself with fresh eyes later. I like to be especially careful with any changes dealing with security.

Nice job figuring out the codebase enough to make the changes you did, btw!

@KutnerUri
Copy link
Contributor Author

KutnerUri commented Oct 13, 2021

thanks, waiting for a new version of penpal! 😃

I think we should document the "*" option in the JSDocs. (Use "*" to skip origin check).
The test is also imperfect - setting the frame src after connectToChild() was a hack to make it fail. I'd rather use a different server url to make the test fail when not using "*" (but it was complicated for me to implement).

@Aaronius
Copy link
Owner

Yeah, I agree. I'm cooking up those changes now. 🧑‍🍳

@Aaronius Aaronius merged commit f2e090b into Aaronius:master Oct 14, 2021
@Aaronius
Copy link
Owner

Thanks for your contribution, @KutnerUri! Released as 6.2.0.

@KutnerUri KutnerUri deleted the null-skip-origin-check branch October 14, 2021 17:30
@KutnerUri
Copy link
Contributor Author

thanks! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dynamic iframe url
2 participants