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

added configuration from remote side #7

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

DaniHaag
Copy link

With this change it is possible to allow the parent to define the instanceId of the iframe. It is also possible to define the filter configuration the same way.
@see #6

@DaniHaag
Copy link
Author

Hi,
this request is now 6 months old and it has not even been commented on.
What is the reason for this?
Is the description unclear or is it not a valid request?

@ifandelse
Copy link
Contributor

@DaniHaag I apologize, but this slipped through the cracks. What's the reason? Well, my 3rd son was born recently, I homeschool 2 other kids, I work 2 jobs most of the time and I maintain several other OSS projects - and try to sleep somewhere in the midst of all that. I didn't intentionally neglect your PR, it honestly slipped through the cracks. Now that I'm aware, I will try and set aside some time to review it. I would ask that you be patient, considering the list I just shared. :-)

@DaniHaag
Copy link
Author

Hi Jim,
No need to apologize. I did not want to be rude, I was just wondering why there was no answer at all, I thought it might be due to my bad explanation. I know that it is hard to keep work up on open source projects. I am happy that you will have a look at my changes. Please let me know, if you need more information. or if you are not content with the quality of the code.

@ifandelse
Copy link
Contributor

Hi @DaniHaag Just wanted to let you know I've finally started squeezing time in at night here to try and look over things. I definitely like the idea of being able to pass a remote config. In fact, @dcneiner and I have talked about things along these lines before - I'm curious if he has any thoughts as well...

@dcneiner
Copy link
Contributor

Hey @ifandelse and @DaniHaag – Just looked over this request in detail and have a couple questions/thoughts:

  1. Why do you need to explicitly set the instanceId of another frame's postal instance. I ask, because if I understood the problem this was meant to solve, I might be able to make more relevant feedback.
  2. Allowing filters to be set from a remote endpoint would be a change with potential security implications (at least it would impact how I use postal.federation), so we may want to consider making the default of restrictRemoteConfig to return false and it could be overridden to enable it.

I have explored being able to give remote frames "roles" where filters are specified on a per role instance, so it is similar in some ways to defining roles remotely.

@DaniHaag
Copy link
Author

@dcneiner My use case is like this. I have an outer iframe and 1 to n inner iframes. It is even possible to have the same iframe in the screen more than once (with different url) parameters.
The iframes should know as little as possible of the surrounding document.
I sometimes need to adress/identify exactly one iframe eg for dynamic resizing according to the size sent by the iframe. If the outer document is not able to configure the iframes it si difficult to keep track of the mapping between iframe ids and postal instances.

About the security considerations, I am totally with you to set the default to the more secure option.

@dcneiner
Copy link
Contributor

Thanks @DaniHaag! Sorry its taken so long to respond, I've unfortunately been quite sick this week :(

Even with these changes, how do you ensure your parent page wires up these children correctly? Are you matching the sources against a known list of iframes or something? To clarify, how does your parent page know to give iframeA1 the instance id of A01 and iframeA5 the instance id of A05 if other than a few query parameters both iframes are the same?

I know how I've solved this problem, but it involved quite a bit of hacking I hope to contribute back soon. I used custom data role attributes on each iframe and then my code used that to wire them up to the correct filters, etc.

@ifandelse
Copy link
Contributor

@DaniHaag & @dcneiner Apologies for taking a bit. I've been catching up on several tasks related to postal & postal add-ons as well as machina. I definitely need more info and input on this. Here are my concerns:

  • Allowing a remote instance to be configured also provides a means to hijack any postal instance. I realize that the PR include a means to disable remote configuration, but allowing this kind of remote config from postal itself is a huge risk (I have alternative suggestions below).
  • I believe that a means to do this already exists with the current functionality - but it does not involve postal/postal.federation to send the configuration data.
  • I understand the desire for the iframes to not be required to know anything about the environment they are being loaded into, but the instanceId is intended to be controlled by the local instance, and this still fits within the iframe not needing to know about the hosting page, etc.
  • I share @dcneiner's question as to how you're currently keeping track of which iframe is which if you are manually setting the IDs. Are you loading them in a serial fashion?

I've faced situations similar to this, where a parent page loaded several 3rd party widgets into iframes. One means that is immediately obvious to me is that if you need the parent to be able to set a child postal instanceId, then pass it on the url to the iframe and have infrastructure code in your iframe that parses the value off of the query string and passes it to postal.instanceId(parsedFromQryString);. That way the remote instance wouldn't be open to hijacking from the console (or from another malicious script), and the ID would be set as the iframe loads, outside postal/postal.federation.

Another approach - though it involves a bit more "handshaking" - would be to have the child frame ask for remote configuration data. This is possible today if you hand roll your own approach.... you could ensure that both iframe and parent have filters allowing a (for example) "configuration" channel to cross back and forth. The child iframe could send a request.config message topic, to which the parent window is listening and would then reply with the correct information for that child iframe, etc.
By making the choice to ask for config data, rather than open it up to be pushed into the client, we've alleviated one of the main security concerns I have with allowing remote configuration.

I still need to look over @dcneiner's code in depth as well - since he's doing some interesting work in allowing "roles/modes" to be assigned to remote instances as they federate (this sets up local filters on how to talk to that remote instance, etc.) - depending on what he and I generalize there, it may land in postal.federation, or in a federation add-on. If you both think having a formalized federation message type for asking for remote configuration data from the child is worth looking into, I can add that to my tasks to investigate. Regardless, though, this kind of behavior is possible now, I believe, if you use one of the approaches I mentioned above.....

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.

3 participants