search

lookbook-hq / lookbook

A UI development environment for Ruby on Rails apps ✨
https://lookbook.build
MIT License
912 stars 94 forks source link

Support configurable frame ancestors for embeds #571

Open aduth opened 11 months ago

aduth commented 11 months ago

Is your feature request related to a problem? Please describe

Currently, it is only possible to configure Lookbook to allow embeds from the same origin or for all external sites.

Ideally, it'd be possible to configure specific hosts from which Lookbook can be embedded, using the frame-ancestors Content-Security-Policy.

The existing option for preview_embeds.policy controls X-Frame-Options which, as noted at the MDN article, is largely obsolete in favor of this frame-ancestors CSP directive.

It's also not possible for us to manually add a policy revision to add this CSP directive, since Lookbook disables the content security policy altogether. We've found a way to work around this in our project, but it involves creating a middleware to modify the raw response headers, which is less than ideal.

Describe the solution you'd like

A new option like config.lookbook.preview_embeds.frame_ancestors that accepts an array of sources to be included in a frame-ancestors CSP directive controlled by Lookbook. If present, it may make sense for this to either take precedent over the X-Frame-Options behavior controlled by preview_embeds.policy, or set the value of preview_embeds.policy to ALLOWALL.

Alternatively, if Lookbook were not to disable the content security policy outright and instead modify it as necessary for its own operation (as discussed in #202), it would allow a downstream project to define their own frame-ancestors policy.

allmarkedup commented 9 months ago

Hey @aduth, thanks for this and many apologies for taking so long to reply.

I have to say my knowledge of CSPs is still pretty limited (despite your help in #202) but what you are suggesting makes total sense.

My gut reaction is to go down the preview_embeds.frame_ancestors config option route as I still feel that keeping parity with the way that ViewComponent disables the CSP is probably still preferable. I'm also a little nervous about adding more complex policy configuration code when I don't feel like I have a lot of expertise in this area.

I'm pretty short of time for working on Lookbook at the moment but I'll try to have a proper think about this as soon as I can. As the config option route is probably simpler I may see if I can give that a go - I'll let you know once I have any code ready so you can take a look at it and see if it will work for you.

aduth commented 9 months ago

Hi @allmarkedup, totally understand if you don't have the bandwidth for this at the moment! Fortunately our workaround is serving us just fine for the time being, but we'll look forward to this functionality being supported out of the box. Happy to review or provide feedback for any changes or ideas you come up with.