Merge branch 'extract-security-dashboard-permissions' into 'master'

Extract security dashboard permissions

See merge request gitlab-org/gitlab-ee!14754

ee/app/controllers/concerns/security_dashboards_permissions.rb

+# frozen_string_literal: true
+
+module SecurityDashboardsPermissions
+  extend ActiveSupport::Concern
+
+  included do
+    before_action :ensure_security_dashboard_feature_enabled!
+    before_action :authorize_read_security_dashboard!
+  end
+
+  protected
+
+  def ensure_security_dashboard_feature_enabled!
+    render_404 unless vulnerable.feature_available?(:security_dashboard)
+  end
+
+  def authorize_read_security_dashboard!
+    render_403 unless can?(current_user, read_security_dashboard, vulnerable)
+  end
+
+  def read_security_dashboard
+    "read_#{vulnerable.class.name.downcase}_security_dashboard".to_sym
+  end
+end

ee/app/controllers/groups/security/application_controller.rb

-# frozen_string_literal: true
-class Groups::Security::ApplicationController < Groups::ApplicationController
-  before_action :ensure_security_dashboard_feature_enabled!
-  before_action :authorize_read_group_security_dashboard!
-
-  protected
-
-  def ensure_security_dashboard_feature_enabled!
-    render_404 unless group.feature_available?(:security_dashboard)
-  end
-
-  def authorize_read_group_security_dashboard!
-    render_403 unless helpers.can_read_group_security_dashboard?(group)
-  end
-end

ee/app/controllers/groups/security/dashboard_controller.rb

 # frozen_string_literal: true
-class Groups::Security::DashboardController < Groups::Security::ApplicationController
+class Groups::Security::DashboardController < Groups::ApplicationController
   layout 'group'
 
-  skip_before_action :ensure_security_dashboard_feature_enabled!, only: [:show]
-  skip_before_action :authorize_read_group_security_dashboard!, only: [:show]
-
   def show
     render :unavailable unless dashboard_available?
   end
@@ -13,6 +10,6 @@ class Groups::Security::DashboardController < Groups::Security::ApplicationContr
 
   def dashboard_available?
     group.feature_available?(:security_dashboard) &&
-      helpers.can_read_group_security_dashboard?(group)
+      can?(current_user, :read_group_security_dashboard, group)
   end
 end

ee/app/controllers/groups/security/vulnerabilities_controller.rb

 # frozen_string_literal: true
 
-class Groups::Security::VulnerabilitiesController < Groups::Security::ApplicationController
+class Groups::Security::VulnerabilitiesController < Groups::ApplicationController
+  include SecurityDashboardsPermissions
   include VulnerabilitiesActions
 
-  private
-
-  def vulnerable
-    group
-  end
+  alias_method :vulnerable, :group
 end

ee/app/controllers/projects/security/dashboard_controller.rb

@@ -3,19 +3,14 @@
 module Projects
   module Security
     class DashboardController < Projects::ApplicationController
-      before_action :ensure_security_dashboard_feature_enabled
-      before_action :authorize_read_project_security_dashboard!
+      include SecurityDashboardsPermissions
+
+      alias_method :vulnerable, :project
 
       def show
         @pipeline = @project.latest_pipeline_with_security_reports
           &.present(current_user: current_user)
       end
-
-      private
-
-      def ensure_security_dashboard_feature_enabled
-        render_404 unless @project.feature_available?(:security_dashboard)
-      end
     end
   end
 end

ee/app/helpers/ee/preferences_helper.rb

@@ -3,7 +3,6 @@
 module EE
   module PreferencesHelper
     extend ::Gitlab::Utils::Override
-    include ::Groups::Security::DashboardHelper
 
     override :excluded_dashboard_choices
     def excluded_dashboard_choices

ee/app/helpers/groups/security/dashboard_helper.rb

-# frozen_string_literal: true
-
-module Groups
-  module Security
-    module DashboardHelper
-      def can_read_group_security_dashboard?(group)
-        can?(current_user, :read_group_security_dashboard, group)
-      end
-    end
-  end
-end

ee/spec/controllers/groups/security/vulnerabilities_controller_spec.rb

@@ -11,46 +11,13 @@ describe Groups::Security::VulnerabilitiesController do
     let(:vulnerable_params) { { group_id: group } }
   end
 
-  before do
-    sign_in(user)
+  it_behaves_like SecurityDashboardsPermissions do
+    let(:vulnerable) { group }
+    let(:security_dashboard_action) { get :index, params: { group_id: group }, format: :json }
   end
 
-  describe 'access for all actions' do
-    context 'when security dashboard feature is disabled' do
-      it 'returns 404' do
-        stub_licensed_features(security_dashboard: false)
-
-        get :index, params: { group_id: group }, format: :json
-
-        expect(response).to have_gitlab_http_status(404)
-      end
-    end
-
-    context 'when security dashboard feature is enabled' do
-      before do
-        stub_licensed_features(security_dashboard: true)
-      end
-
-      context 'when user has guest access' do
-        it 'denies access' do
-          group.add_guest(user)
-
-          get :index, params: { group_id: group }, format: :json
-
-          expect(response).to have_gitlab_http_status(403)
-        end
-      end
-
-      context 'when user has developer access' do
-        it 'grants access' do
-          group.add_developer(user)
-
-          get :index, params: { group_id: group }, format: :json
-
-          expect(response).to have_gitlab_http_status(200)
-        end
-      end
-    end
+  before do
+    sign_in(user)
   end
 
   describe 'GET index.json' do

ee/spec/controllers/projects/security/dashboard_controller_spec.rb

@@ -5,6 +5,14 @@ describe Projects::Security::DashboardController do
   set(:project) { create(:project, :repository, :public, namespace: group) }
   set(:user)    { create(:user) }
 
+  it_behaves_like SecurityDashboardsPermissions do
+    let(:vulnerable) { project }
+
+    let(:security_dashboard_action) do
+      get :show, params: { namespace_id: project.namespace, project_id: project }
+    end
+  end
+
   before do
     group.add_developer(user)
   end
@@ -15,81 +23,46 @@ describe Projects::Security::DashboardController do
     render_views
 
     def show_security_dashboard(current_user = user)
+      stub_licensed_features(security_dashboard: true)
       sign_in(current_user)
       get :show, params: { namespace_id: project.namespace, project_id: project }
     end
 
-    context 'when security dashboard feature is enabled' do
+    context 'when uses legacy reports syntax' do
       before do
-        stub_licensed_features(security_dashboard: true)
-      end
-
-      context 'when uses legacy reports syntax' do
-        before do
-          create(:ci_build, :artifacts, pipeline: pipeline, name: 'sast')
-        end
-
-        it 'returns the latest pipeline with security reports for project' do
-          show_security_dashboard
-
-          expect(response).to have_gitlab_http_status(200)
-          expect(response).to render_template(:show)
-          expect(response.body).to have_css("div#js-security-report-app[data-has-pipeline-data=true]")
-        end
-      end
-
-      context 'when uses new reports syntax' do
-        before do
-          create(:ee_ci_build, :sast, pipeline: pipeline)
-        end
-
-        it 'returns the latest pipeline with security reports for project' do
-          show_security_dashboard
-
-          expect(response).to have_gitlab_http_status(200)
-          expect(response).to render_template(:show)
-          expect(response.body).to have_css("div#js-security-report-app[data-has-pipeline-data=true]")
-        end
+        create(:ci_build, :artifacts, pipeline: pipeline, name: 'sast')
       end
 
-      context 'when there is no matching pipeline' do
-        it 'renders empty state' do
-          show_security_dashboard
+      it 'returns the latest pipeline with security reports for project' do
+        show_security_dashboard
 
-          expect(response).to have_gitlab_http_status(200)
-          expect(response).to render_template(:show)
-          expect(response.body).to have_css("div#js-security-report-app[data-has-pipeline-data=false]")
-        end
+        expect(response).to have_gitlab_http_status(200)
+        expect(response).to render_template(:show)
+        expect(response.body).to have_css("div#js-security-report-app[data-has-pipeline-data=true]")
       end
     end
 
-    context 'when security dashboard feature is disabled' do
+    context 'when uses new reports syntax' do
       before do
-        stub_licensed_features(security_dashboard: false)
+        create(:ee_ci_build, :sast, pipeline: pipeline)
       end
 
-      it 'returns 404' do
+      it 'returns the latest pipeline with security reports for project' do
         show_security_dashboard
 
-        expect(response).to have_gitlab_http_status(404)
-        expect(response).to render_template('errors/not_found')
+        expect(response).to have_gitlab_http_status(200)
+        expect(response).to render_template(:show)
+        expect(response.body).to have_css("div#js-security-report-app[data-has-pipeline-data=true]")
       end
     end
 
-    context 'with unauthorized user for security dashboard' do
-      let(:guest) { create(:user) }
-
-      before do
-        stub_licensed_features(security_dashboard: true)
-      end
-
-      it 'returns a not found 404 response' do
-        group.add_guest(guest)
-
-        show_security_dashboard guest
+    context 'when there is no matching pipeline' do
+      it 'renders empty state' do
+        show_security_dashboard
 
-        expect(response).to have_gitlab_http_status(404)
-        expect(response).to render_template('errors/not_found')
+        expect(response).to have_gitlab_http_status(200)
+        expect(response).to render_template(:show)
+        expect(response.body).to have_css("div#js-security-report-app[data-has-pipeline-data=false]")
       end
     end
   end

ee/spec/support/shared_examples/controllers/concerns/security_dashboards_permissions_shared_examples.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+shared_examples SecurityDashboardsPermissions do
+  include ApiHelpers
+
+  let(:security_dashboard_user) { create(:user) }
+
+  before do
+    sign_in(security_dashboard_user)
+  end
+
+  describe 'access for all actions' do
+    context 'when security dashboard feature is disabled' do
+      it 'returns 404' do
+        stub_licensed_features(security_dashboard: false)
+
+        security_dashboard_action
+
+        expect(response).to have_gitlab_http_status(404)
+      end
+    end
+
+    context 'when security dashboard feature is enabled' do
+      before do
+        stub_licensed_features(security_dashboard: true)
+      end
+
+      context 'when user has guest access' do
+        it 'denies access' do
+          vulnerable.add_guest(security_dashboard_user)
+
+          security_dashboard_action
+
+          expect(response).to have_gitlab_http_status(403)
+        end
+      end
+
+      context 'when user has developer access' do
+        it 'grants access' do
+          vulnerable.add_developer(security_dashboard_user)
+
+          security_dashboard_action
+
+          expect(response).to have_gitlab_http_status(200)
+        end
+      end
+    end
+  end
+end

ee/spec/views/profiles/preferences/show.html.haml_spec.rb

@@ -6,7 +6,6 @@ describe 'profiles/preferences/show' do
   before do
     assign(:user, user)
     allow(controller).to receive(:current_user).and_return(user)
-    view.extend ::Groups::Security::DashboardHelper
   end
 
   let(:user) { build(:user) }