Rework <iframe> embed configuration
After a really thorough review and discussion with AppSec ([staff-only link to discussion](https://gitlab.com/gitlab-com/gl-security/product-security/appsec/appsec-reviews/-/work_items/326)), we have a path for `<iframe>` embeds to be GA'd without introducing unacceptable risks. The main proposed changes from the above discussion follow: --- … So, let's change that list of transforms instead to a provider configuration, say `config/iframe_providers.yml`, or `config/markdown_embed_providers.yml`! It replaces (becomes) the allowlist, too. Here's an initial mockup of what that configuration could look like: ```yaml - id: youtube name: YouTube matches: - host: www.youtube.com path: /embed/{v} - host: www.youtube.com path: /watch params: [v] - host: youtu.be path: /{v} src: 'https://www.youtube.com/embed/{v}' sandbox: 'allow-scripts allow-popups allow-same-origin allow-popups-to-escape-sandbox' require-activation: false # default is 'true' - id: figma name: Figma matches: - host: embed.figma.com path: /{type}/{id} - host: www.figma.com path: /{type}/{id} require: type: [proto, design, board, file, slides] src: 'https://embed.figma.com/{type}/{id}?embed-host=gitlab' sandbox: 'allow-scripts allow-popups allow-same-origin' require-activation: true ``` This preserves all the capabilities of the current iteration ("do what I mean"-type embeds, as well as pasted embed codes), but allows per-provider sandbox configuration and is explicit about what URL is actually embedded per-provider, rather than allowing whole domains and just transforming some particular matches as a convenience. The `id` is the key for per-tenant configuration (so the above configuration becomes two checkboxes at the tenant/group/project settings level); we then allow only those enabled to actually be rendered as iframes, with CSP likewise derived from the `src` of enabled providers. Click-to-activate, external content warnings, and other future per-provider settings likewise can be configured here, and changes to the file can be gated on AppSec reviews. While changing the provider config gets all those gates and versioning, toggling providers is still something that would happen with an instance/group/project admin changing a setting. We also need to think about how this works in terms of cascading: can a group *disable* providers such that subgroups/projects *cannot* switch them on? Presumably this is very desireable. (Potentially a group may also wish to _allow_ sub-namespaces to turn on providers without actually enabling it _at_ the group-level? e.g. group wiki) --- I've included the above here to heighten transparency :transparency:; there are more targeted/specific AppSec findings and questions to consider in that thread that need to be considered as part of this work, however.
issue