diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-02-18 09:09:26 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-02-18 09:09:26 +0300 |
commit | eb6f2239b44c99f795d0f614324c0804c556b3d5 (patch) | |
tree | 798d7c062ca5a02e1dcb18bc3c628dc54c86ec34 | |
parent | 1f39f07db14d2525d305a824c0b390e96020aac6 (diff) |
Add latest changes from gitlab-org/gitlab@master
7 files changed, 248 insertions, 103 deletions
diff --git a/app/views/layouts/_loading_hints.html.haml b/app/views/layouts/_loading_hints.html.haml index 9026bec84c3..b20b95cade8 100644 --- a/app/views/layouts/_loading_hints.html.haml +++ b/app/views/layouts/_loading_hints.html.haml @@ -12,7 +12,8 @@ = preload_link_tag(path_to_stylesheet('application'), crossorigin: css_crossorigin) = preload_link_tag(path_to_stylesheet("highlight/themes/#{user_color_scheme}"), crossorigin: css_crossorigin) - if Gitlab::Tracking.enabled? && Gitlab::Tracking.collector_hostname - %link{ rel: 'preconnect', href: "https://#{Gitlab::Tracking.collector_hostname}", crossorigin: '' } + - unless Rails.env.development? + %link{ rel: 'preconnect', href: "https://#{Gitlab::Tracking.collector_hostname}", crossorigin: '' } -# Do not use preload_link_tag for fonts, to work around Firefox double-fetch bug. -# See https://github.com/web-platform-tests/wpt/pull/36930 %link{ rel: 'preload', href: font_path('gitlab-sans/GitLabSans.woff2'), as: 'font', crossorigin: css_crossorigin } diff --git a/lib/banzai/filter/inline_observability_redactor_filter.rb b/lib/banzai/filter/inline_observability_redactor_filter.rb new file mode 100644 index 00000000000..8cd1084e904 --- /dev/null +++ b/lib/banzai/filter/inline_observability_redactor_filter.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +module Banzai + module Filter + class InlineObservabilityRedactorFilter < HTML::Pipeline::Filter + include Gitlab::Utils::StrongMemoize + + CSS_SELECTOR = '.js-render-observability' + + XPATH = Gitlab::Utils::Nokogiri.css_to_xpath(CSS_SELECTOR).freeze + EMBED_LIMIT = 100 + + def call + return doc if Gitlab::Utils.to_boolean(ENV.fetch('STANDALONE_OBSERVABILITY_UI', false)) + + nodes.each do |node| + group_id = group_ids_by_nodes[node] + user_has_access = group_id && user_access_by_group_id[group_id] + node.remove unless user_has_access + end + + doc + end + + private + + def user + context[:current_user] + end + + # Returns all observability embed placeholder nodes + # + # Removes any nodes beyond the first 100 + # + # @return [Nokogiri::XML::NodeSet] + def nodes + nodes = doc.xpath(XPATH) + nodes.drop(EMBED_LIMIT).each(&:remove) + nodes + end + strong_memoize_attr :nodes + + # Returns a mapping representing whether the current user has permission to access observability + # for group-ids linked in by the embed nodes + # + # @return [Hash<String, Boolean>] + def user_access_by_group_id + user_groups_from_nodes.each_with_object({}) do |group, user_access| + user_access[group.id] = Gitlab::Observability.allowed?(user, group, :read_observability) + end + end + strong_memoize_attr :user_access_by_group_id + + # Maps a node to the group_id linked by the node + # + # @return [Hash<Nokogiri::XML::Node, string>] + def group_ids_by_nodes + nodes.each_with_object({}) do |node, group_ids| + url = node.attribute('data-frame-url').to_s + next unless url + + group_id = Gitlab::Observability.group_id_from_url(url) + group_ids[node] = group_id if group_id + end + end + strong_memoize_attr :group_ids_by_nodes + + # Returns the list of groups linked in the embed nodes and readable by the user + # + # @return [ActiveRecord_Relation] + def user_groups_from_nodes + GroupsFinder.new(user, filter_group_ids: group_ids_by_nodes.values.uniq).execute + end + strong_memoize_attr :user_groups_from_nodes + end + end +end diff --git a/lib/banzai/pipeline/post_process_pipeline.rb b/lib/banzai/pipeline/post_process_pipeline.rb index f8035698b9b..b9cf5b4fbdd 100644 --- a/lib/banzai/pipeline/post_process_pipeline.rb +++ b/lib/banzai/pipeline/post_process_pipeline.rb @@ -16,6 +16,7 @@ module Banzai [ Filter::ReferenceRedactorFilter, Filter::InlineMetricsRedactorFilter, + Filter::InlineObservabilityRedactorFilter, # UploadLinkFilter must come before RepositoryLinkFilter to # prevent unnecessary Gitaly calls from being made. Filter::UploadLinkFilter, diff --git a/spec/features/markdown/observability_spec.rb b/spec/features/markdown/observability_spec.rb index 86caf3eb1b1..0b380c74777 100644 --- a/spec/features/markdown/observability_spec.rb +++ b/spec/features/markdown/observability_spec.rb @@ -2,90 +2,50 @@ require 'spec_helper' -RSpec.describe 'Observability rendering', :js do +RSpec.describe 'Observability rendering', :js, feature_category: :metrics do let_it_be(:group) { create(:group, :public) } let_it_be(:project) { create(:project, :repository, group: group) } let_it_be(:user) { create(:user) } - let_it_be(:observable_url) { "https://observe.gitlab.com/" } + let_it_be(:observable_url) { "https://observe.gitlab.com/#{group.id}/some-dashboard" } let_it_be(:expected) do %(<iframe src="#{observable_url}?theme=light&kiosk" frameborder="0") end before do - project.add_maintainer(user) + group.add_developer(user) sign_in(user) end - context 'when embedding in an issue' do - let(:issue) do - create(:issue, project: project, description: observable_url) - end - - before do - visit project_issue_path(project, issue) - wait_for_requests - end - - it 'renders iframe in description' do - page.within('.description') do - expect(page.html).to include(expected) - end - end - - it 'renders iframe in comment' do - expect(page).not_to have_css('.note-text') - - page.within('.js-main-target-form') do - fill_in('note[note]', with: observable_url) - click_button('Comment') + context 'when user is a developer of the embedded group' do + context 'when embedding in an issue' do + let(:issue) do + create(:issue, project: project, description: observable_url) end - wait_for_requests - - page.within('.note-text') do - expect(page.html).to include(expected) + before do + visit project_issue_path(project, issue) + wait_for_requests end - end - end - context 'when embedding in an MR' do - let(:merge_request) do - create(:merge_request, source_project: project, target_project: project, description: observable_url) - end - - before do - visit merge_request_path(merge_request) - wait_for_requests + it_behaves_like 'embeds observability' end - it 'renders iframe in description' do - page.within('.description') do - expect(page.html).to include(expected) + context 'when embedding in an MR' do + let(:merge_request) do + create(:merge_request, source_project: project, target_project: project, description: observable_url) end - end - it 'renders iframe in comment' do - expect(page).not_to have_css('.note-text') - - page.within('.js-main-target-form') do - fill_in('note[note]', with: observable_url) - click_button('Comment') + before do + visit merge_request_path(merge_request) + wait_for_requests end - wait_for_requests - - page.within('.note-text') do - expect(page.html).to include(expected) - end + it_behaves_like 'embeds observability' end end - context 'when feature flag is disabled' do - before do - stub_feature_flags(observability_group_tab: false) - end - + shared_examples 'does not embed observability in issues and MRs' do context 'when embedding in an issue' do let(:issue) do create(:issue, project: project, description: observable_url) @@ -96,28 +56,7 @@ RSpec.describe 'Observability rendering', :js do wait_for_requests end - it 'does not render iframe in description' do - page.within('.description') do - expect(page.html).not_to include(expected) - expect(page.html).to include(observable_url) - end - end - - it 'does not render iframe in comment' do - expect(page).not_to have_css('.note-text') - - page.within('.js-main-target-form') do - fill_in('note[note]', with: observable_url) - click_button('Comment') - end - - wait_for_requests - - page.within('.note-text') do - expect(page.html).not_to include(expected) - expect(page.html).to include(observable_url) - end - end + it_behaves_like 'does not embed observability' end context 'when embedding in an MR' do @@ -130,28 +69,21 @@ RSpec.describe 'Observability rendering', :js do wait_for_requests end - it 'does not render iframe in description' do - page.within('.description') do - expect(page.html).not_to include(expected) - expect(page.html).to include(observable_url) - end - end - - it 'does not render iframe in comment' do - expect(page).not_to have_css('.note-text') - - page.within('.js-main-target-form') do - fill_in('note[note]', with: observable_url) - click_button('Comment') - end + it_behaves_like 'does not embed observability' + end + end - wait_for_requests + context 'when user is not a developer of the embeded group' do + it_behaves_like 'does not embed observability in issues and MRs' do + let_it_be(:observable_url) { "https://observe.gitlab.com/1234/some-dashboard" } + end + end - page.within('.note-text') do - expect(page.html).not_to include(expected) - expect(page.html).to include(observable_url) - end - end + context 'when feature flag is disabled' do + before do + stub_feature_flags(observability_group_tab: false) end + + it_behaves_like 'does not embed observability in issues and MRs' end end diff --git a/spec/lib/banzai/filter/inline_observability_redactor_filter_spec.rb b/spec/lib/banzai/filter/inline_observability_redactor_filter_spec.rb new file mode 100644 index 00000000000..ceebe42d1b7 --- /dev/null +++ b/spec/lib/banzai/filter/inline_observability_redactor_filter_spec.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Banzai::Filter::InlineObservabilityRedactorFilter, feature_category: :metrics do + include FilterSpecHelper + + let_it_be(:group) { create(:group) } + + let(:url) { "#{Gitlab::Observability.observability_url}/#{group.id}/explore" } + let(:input) { %(<a href="#{url}">example</a>) } + let(:doc) { filter(input) } + + context 'without an observability placeholder' do + it 'leaves regular links unchanged' do + expect(doc.to_s).to eq input + end + end + + shared_examples 'redacts the placeholder' do + it 'redacts the placeholder' do + expect(doc.to_s).to be_empty + end + end + + context 'with an observability placeholder' do + let(:input) { %(<div class="js-render-observability" data-frame-url="#{url}"></div>) } + + context 'when no user is logged in' do + it_behaves_like 'redacts the placeholder' + end + + context 'with invalid observability url' do + let(:url) { "#{Gitlab::Observability.observability_url}/foo/explore" } + + it_behaves_like 'redacts the placeholder' + end + + context 'with missing observability frame url' do + let(:input) { %(<div class="js-render-observability"></div>) } + + it_behaves_like 'redacts the placeholder' + end + + context 'when the user does not have permission to access the group' do + let(:user) { create(:user) } + let(:doc) { filter(input, current_user: user) } + + it_behaves_like 'redacts the placeholder' + end + + context 'when the user is not a developer of the group' do + let(:user) { create(:user) } + let(:doc) { filter(input, current_user: user) } + + before do + group.add_reporter(user) + end + + it_behaves_like 'redacts the placeholder' + end + + context 'when the user is a developer of the group' do + let(:user) { create(:user) } + let(:doc) { filter(input, current_user: user) } + + before do + group.add_developer(user) + end + + it 'leaves the placeholder' do + expect(CGI.unescapeHTML(doc.to_s)).to eq(input) + end + + context 'with over 100 embeds' do + let(:embed) { %(<div class="js-render-observability" data-frame-url="#{url}"></div>) } + let(:input) { embed * 150 } + + it 'redacts ill-advised embeds' do + expect(doc.to_s.length).to eq(embed.length * 100) + end + end + end + end +end diff --git a/spec/lib/banzai/pipeline/post_process_pipeline_spec.rb b/spec/lib/banzai/pipeline/post_process_pipeline_spec.rb index 072d77f4112..c8d3844af30 100644 --- a/spec/lib/banzai/pipeline/post_process_pipeline_spec.rb +++ b/spec/lib/banzai/pipeline/post_process_pipeline_spec.rb @@ -36,7 +36,7 @@ RSpec.describe Banzai::Pipeline::PostProcessPipeline, feature_category: :team_pl end let(:doc) { HTML::Pipeline.parse(html) } - let(:non_related_xpath_calls) { 2 } + let(:non_related_xpath_calls) { 3 } it 'searches for attributes only once' do expect(doc).to receive(:xpath).exactly(non_related_xpath_calls + 1).times diff --git a/spec/support/shared_examples/observability/embed_observabilities_examples.rb b/spec/support/shared_examples/observability/embed_observabilities_examples.rb new file mode 100644 index 00000000000..27a8bd614e3 --- /dev/null +++ b/spec/support/shared_examples/observability/embed_observabilities_examples.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'embeds observability' do + it 'renders iframe in description' do + page.within('.description') do + expect(page.html).to include(expected) + end + end + + it 'renders iframe in comment' do + expect(page).not_to have_css('.note-text') + + page.within('.js-main-target-form') do + fill_in('note[note]', with: observable_url) + click_button('Comment') + end + + wait_for_requests + + page.within('.note-text') do + expect(page.html).to include(expected) + end + end +end + +RSpec.shared_examples 'does not embed observability' do + it 'does not render iframe in description' do + page.within('.description') do + expect(page.html).not_to include(expected) + expect(page.html).to include(observable_url) + end + end + + it 'does not render iframe in comment' do + expect(page).not_to have_css('.note-text') + + page.within('.js-main-target-form') do + fill_in('note[note]', with: observable_url) + click_button('Comment') + end + + wait_for_requests + + page.within('.note-text') do + expect(page.html).not_to include(expected) + expect(page.html).to include(observable_url) + end + end +end |