diff --git a/app/models/group.rb b/app/models/group.rb
index 4f717613675d9ea37983714a99c9112836d5a8fd..c63b02cfa1914241f1c2125c614e9bc91e6f45ab 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -1177,6 +1177,10 @@ def glql_load_on_click_feature_flag_enabled?
feature_flag_enabled_for_self_or_ancestor?(:glql_load_on_click, type: :ops)
end
+ def allow_iframes_in_markdown_feature_flag_enabled?
+ feature_flag_enabled_for_self_or_ancestor?(:allow_iframes_in_markdown, type: :wip)
+ end
+
# overriden in EE
def supports_group_work_items?
false
diff --git a/app/models/project.rb b/app/models/project.rb
index c5534f99395566767861d9275a29d07f35f49c6d..a09925cbcda2f4274d566dbd075414cc33b4a057 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -3510,6 +3510,10 @@ def markdown_placeholders_feature_flag_enabled?
group&.markdown_placeholders_feature_flag_enabled? || Feature.enabled?(:markdown_placeholders, self, type: :gitlab_com_derisk)
end
+ def allow_iframes_in_markdown_feature_flag_enabled?
+ group&.allow_iframes_in_markdown_feature_flag_enabled? || Feature.enabled?(:allow_iframes_in_markdown, self, type: :wip)
+ end
+
def enqueue_record_project_target_platforms
return unless Gitlab.com?
diff --git a/config/feature_flags/wip/allow_iframes_in_markdown.yml b/config/feature_flags/wip/allow_iframes_in_markdown.yml
new file mode 100644
index 0000000000000000000000000000000000000000..faf41918f0424703486d71e081a0545b4d2b6128
--- /dev/null
+++ b/config/feature_flags/wip/allow_iframes_in_markdown.yml
@@ -0,0 +1,10 @@
+---
+name: allow_iframes_in_markdown
+description:
+feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/282443
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/204952
+rollout_issue_url:
+milestone: '18.5'
+group: group::knowledge
+type: wip
+default_enabled: false
diff --git a/lib/banzai/filter/iframe_link_filter.rb b/lib/banzai/filter/iframe_link_filter.rb
new file mode 100644
index 0000000000000000000000000000000000000000..5c355c5b502d6de0a5d060f1eb39d79f46ec46c3
--- /dev/null
+++ b/lib/banzai/filter/iframe_link_filter.rb
@@ -0,0 +1,51 @@
+# frozen_string_literal: true
+
+# Determines if an `img` tag references media to be embedded in an `iframe`. The administrator
+# needs to explicitly allow the domain and consider it trusted. The `js-render-iframe` class
+# will get added to allow the frontend to convert into an `iframe`
+#
+# Even though the `iframe` src will have been allowed by the administrator, don't insert
+# the `iframe` tag here on the backend - allow the frontend to handle it. This allows for
+# the administrator to remove the domain in the future if it becomes untrusted for some reason.
+# The markdown cache will not need to cleared as long as the `iframe` is added on the frontend.
+module Banzai
+ module Filter
+ class IframeLinkFilter < PlayableLinkFilter
+ extend ::Gitlab::Utils::Override
+
+ private
+
+ def media_type
+ 'img'
+ end
+
+ def safe_media_ext
+ # TODO: will change to use the administrator defined allow list
+ # Gitlab::CurrentSettings.iframe_src_allowlist
+ ['www.youtube.com/embed']
+ end
+
+ override :has_allowed_media?
+ def has_allowed_media?(element)
+ return unless context[:project]&.allow_iframes_in_markdown_feature_flag_enabled? ||
+ context[:group]&.allow_iframes_in_markdown_feature_flag_enabled?
+
+ src = element.attr('data-canonical-src').presence || element.attr('src')
+
+ return unless src.present?
+
+ src.start_with?('https://') && safe_media_ext.any? { |domain| src.start_with?("https://#{domain}") }
+ end
+
+ def extra_element_attrs(element)
+ attrs = {}
+
+ attrs[:height] = element[:height] if element[:height]
+ attrs[:width] = element[:width] if element[:width]
+ attrs[:class] = 'js-render-iframe'
+
+ attrs
+ end
+ end
+ end
+end
diff --git a/lib/banzai/filter/playable_link_filter.rb b/lib/banzai/filter/playable_link_filter.rb
index 8dc419380e6ba47db711f4a13ff4d63f0f3674a1..c094be78b1586751efd9f07e5d265a0ef275fd1c 100644
--- a/lib/banzai/filter/playable_link_filter.rb
+++ b/lib/banzai/filter/playable_link_filter.rb
@@ -10,7 +10,7 @@ class PlayableLinkFilter < HTML::Pipeline::Filter
def call
doc.xpath('descendant-or-self::img[not(ancestor::a)]').each do |el|
- el.replace(media_node(doc, el)) if has_media_extension?(el)
+ el.replace(media_node(doc, el)) if has_allowed_media?(el)
end
doc
@@ -30,7 +30,7 @@ def extra_element_attrs(element)
{}
end
- def has_media_extension?(element)
+ def has_allowed_media?(element)
src = element.attr('data-canonical-src').presence || element.attr('src')
return unless src.present?
diff --git a/lib/banzai/pipeline/gfm_pipeline.rb b/lib/banzai/pipeline/gfm_pipeline.rb
index 9d0edf01a41910005cf954f1ed517e7d22f78f18..da16276ab9938d14f9f170b813a08bf24715ef89 100644
--- a/lib/banzai/pipeline/gfm_pipeline.rb
+++ b/lib/banzai/pipeline/gfm_pipeline.rb
@@ -29,6 +29,7 @@ def self.filters
Filter::AttributesFilter,
Filter::VideoLinkFilter,
Filter::AudioLinkFilter,
+ Filter::IframeLinkFilter,
Filter::TableOfContentsTagFilter,
Filter::AutolinkFilter,
Filter::SuggestionFilter,
diff --git a/spec/lib/banzai/filter/audio_link_filter_spec.rb b/spec/lib/banzai/filter/audio_link_filter_spec.rb
index 2307b05d63640233a584473cd0bed6dcffc1c171..1059b55d2cdabaf94da47f9d10fedfd66ffd1730 100644
--- a/spec/lib/banzai/filter/audio_link_filter_spec.rb
+++ b/spec/lib/banzai/filter/audio_link_filter_spec.rb
@@ -17,7 +17,7 @@ def link_to_image(path)
%(
)
end
- let(:project) { create(:project, :repository) }
+ let_it_be(:project) { create(:project, :repository) }
shared_examples 'an audio element' do
let(:image) { link_to_image(src) }
diff --git a/spec/lib/banzai/filter/iframe_link_filter_spec.rb b/spec/lib/banzai/filter/iframe_link_filter_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..5ebc97370991b275410b2d8ddebfedc9fb7db200
--- /dev/null
+++ b/spec/lib/banzai/filter/iframe_link_filter_spec.rb
@@ -0,0 +1,142 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Banzai::Filter::IframeLinkFilter, feature_category: :markdown do
+ def filter(doc, contexts = {})
+ contexts.reverse_merge!({ project: project })
+
+ described_class.call(doc, contexts)
+ end
+
+ def link_to_image(path, height = nil, width = nil)
+ return '
' if path.nil?
+
+ attrs = %(src="#{path}")
+ attrs += %( width="#{width}") if width
+ attrs += %( height="#{height}") if height
+
+ %(
)
+ end
+
+ let_it_be(:project) { create(:project, :repository) }
+
+ shared_examples 'an iframe element' do
+ let(:image) { link_to_image(src, height, width) }
+
+ it 'replaces the image tag with a media container and image tag' do
+ container = filter(image).children.first
+
+ expect(container.name).to eq 'span'
+ expect(container['class']).to eq 'media-container img-container'
+
+ iframe, link = container.children
+
+ expect(iframe.name).to eq 'img'
+ expect(iframe['src']).to eq src
+ expect(iframe['height']).to eq height if height
+ expect(iframe['width']).to eq width if width
+
+ expect(link.name).to eq 'a'
+ expect(link['href']).to eq src
+ expect(link['target']).to eq '_blank'
+ end
+ end
+
+ shared_examples 'an unchanged element' do
+ it 'leaves the document unchanged' do
+ element = filter(link_to_image(src)).children.first
+
+ expect(element.name).to eq 'img'
+ expect(element['src']).to eq src
+ end
+ end
+
+ context 'when the element src has a supported iframe domain' do
+ let(:height) { nil }
+ let(:width) { nil }
+
+ it_behaves_like 'an iframe element' do
+ let(:src) { "https://www.youtube.com/embed/foo" }
+ end
+ end
+
+ context 'when the element has height or width specified' do
+ let(:src) { "https://www.youtube.com/embed/foo" }
+
+ it_behaves_like 'an iframe element' do
+ let(:height) { '100%' }
+ let(:width) { '50px' }
+ end
+
+ it_behaves_like 'an iframe element' do
+ let(:height) { nil }
+ let(:width) { '50px' }
+ end
+
+ it_behaves_like 'an iframe element' do
+ let(:height) { '50px' }
+ let(:width) { nil }
+ end
+ end
+
+ context 'when the element has no src attribute' do
+ let(:src) { nil }
+
+ it_behaves_like 'an unchanged element'
+ end
+
+ context 'when the element src does not match a domain' do
+ let(:src) { 'https://path/my_image.jpg' }
+
+ it_behaves_like 'an unchanged element'
+ end
+
+ context 'when data-canonical-src is empty' do
+ let(:image) { %(
) }
+
+ context 'and src is for an iframe' do
+ let(:src) { "https://www.youtube.com/embed/foo" }
+ let(:height) { nil }
+ let(:width) { nil }
+
+ it_behaves_like 'an iframe element'
+ end
+
+ context 'and src is an image' do
+ let(:src) { 'https://path/my_image.jpg' }
+
+ it_behaves_like 'an unchanged element'
+ end
+ end
+
+ context 'when data-canonical-src is set' do
+ it 'uses the correct src' do
+ proxy_src = 'https://assets.example.com/6d8b63'
+ canonical_src = 'https://www.youtube.com/embed/foo'
+ image = %(
)
+ container = filter(image).children.first
+
+ expect(container['class']).to eq 'media-container img-container'
+
+ iframe, link = container.children
+
+ expect(iframe['src']).to eq proxy_src
+ expect(iframe['data-canonical-src']).to eq canonical_src
+
+ expect(link['href']).to eq proxy_src
+ end
+ end
+
+ context 'when allow_iframes_in_markdown is disabled' do
+ let(:src) { 'https://www.youtube.com/embed/foo' }
+
+ before do
+ stub_feature_flags(allow_iframes_in_markdown: false)
+ end
+
+ it_behaves_like 'an unchanged element'
+ end
+
+ it_behaves_like 'pipeline timing check'
+end
diff --git a/spec/lib/banzai/filter/video_link_filter_spec.rb b/spec/lib/banzai/filter/video_link_filter_spec.rb
index 2bf0c692027f74297cf43aa6bd5004716d09b8d2..aec32218b66c4e570fe3cde75e7d02d263cdd2ec 100644
--- a/spec/lib/banzai/filter/video_link_filter_spec.rb
+++ b/spec/lib/banzai/filter/video_link_filter_spec.rb
@@ -21,7 +21,7 @@ def link_to_image(path, height = nil, width = nil)
%(
)
end
- let(:project) { create(:project, :repository) }
+ let_it_be(:project) { create(:project, :repository) }
shared_examples 'a video element' do
let(:image) { link_to_image(src, height, width) }