Add latest changes from gitlab-org/gitlab@master

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 }

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

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,

spec/features/markdown/observability_spec.rb

@@ -2,90 +2,22 @@
 
 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&amp;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')
-      end
-
-      wait_for_requests
-
-      page.within('.note-text') do
-        expect(page.html).to include(expected)
-      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
-    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')
-      end
-
-      wait_for_requests
-
-      page.within('.note-text') do
-        expect(page.html).to include(expected)
-      end
-    end
-  end
-
-  context 'when feature flag is disabled' do
-    before do
-      stub_feature_flags(observability_group_tab: false)
-    end
-
+  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)
@@ -96,28 +28,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 'embeds observability'
     end
 
     context 'when embedding in an MR' do
@@ -130,28 +41,49 @@ 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 'embeds observability'
     end
   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)
+      end
+
+      before do
+        visit project_issue_path(project, issue)
+        wait_for_requests
+      end
+
+      it_behaves_like 'does not embed observability'
+    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
+      end
+
+      it_behaves_like 'does not embed observability'
+    end
+  end
+
+  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
+
+  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

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

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

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