Merge branch '12250-fix-permissions-for-dependency-list' into 'master'

Access to Dependency List page

Closes #12250

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

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

@@ -3,8 +3,8 @@
 module Projects
   module Security
     class DependenciesController < Projects::ApplicationController
-      before_action :authorize_read_dependency_list!
       before_action :ensure_dependency_list_feature_available
+      before_action :authorize_read_dependency_list!
 
       def index
         respond_to do |format|
@@ -33,7 +33,7 @@ module Projects
       end
 
       def authorize_read_dependency_list!
-        return render_403 unless can?(current_user, :read_project_security_dashboard, project)
+        render_403 unless can?(current_user, :read_dependencies, project)
       end
 
       def ensure_dependency_list_feature_available
@@ -61,7 +61,7 @@ module Projects
       end
 
       def serializer
-        serializer = ::DependencyListSerializer.new(project: project)
+        serializer = ::DependencyListSerializer.new(project: project, user: current_user)
         serializer = serializer.with_pagination(request, response) if params[:page]
         serializer
       end

ee/app/serializers/dependency_entity.rb

 # frozen_string_literal: true
 
 class DependencyEntity < Grape::Entity
+  include RequestAwareEntity
+
   class LocationEntity < Grape::Entity
     expose :blob_path, :path
   end
@@ -11,5 +13,11 @@ class DependencyEntity < Grape::Entity
 
   expose :name, :packager, :version
   expose :location, using: LocationEntity
-  expose :vulnerabilities, using: VulnerabilityEntity
+  expose :vulnerabilities, using: VulnerabilityEntity, if: ->(_) { can_read_vulnerabilities? }
+
+  private
+
+  def can_read_vulnerabilities?
+    can?(request.user, :read_project_security_dashboard, request.project)
+  end
 end

ee/app/serializers/dependency_list_entity.rb

@@ -11,13 +11,17 @@ class DependencyListEntity < Grape::Entity
       status(list[:dependencies], options[:build])
     end
 
-    expose :job_path, if: ->(_, options) { options[:build] } do |_, options|
+    expose :job_path, if: ->(_, options) { options[:build] && can_read_job_path? } do |_, options|
       project_build_path(project, options[:build].id)
     end
   end
 
   private
 
+  def can_read_job_path?
+    can?(request.user, :read_pipeline, project)
+  end
+
   def project
     request.project
   end

ee/changelogs/unreleased/12250-fix-permissions-for-dependency-list.yml

+---
+title: Update permissions for Dependency List
+merge_request: 15044
+author:
+type: changed

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

@@ -4,7 +4,7 @@ require 'spec_helper'
 
 describe Projects::Security::DependenciesController do
   describe 'GET index.json' do
-    set(:project) { create(:project, :repository, :public) }
+    set(:project) { create(:project, :repository, :private) }
     set(:user) { create(:user) }
     let(:params) { { namespace_id: project.namespace, project_id: project } }
 
@@ -19,7 +19,7 @@ describe Projects::Security::DependenciesController do
 
       context 'when feature is available' do
         before do
-          stub_licensed_features(dependency_list: true, security_dashboard: true)
+          stub_licensed_features(dependency_list: true)
         end
 
         it 'counts usage of the feature' do
@@ -167,8 +167,6 @@ describe Projects::Security::DependenciesController do
 
       context 'when feature is not available' do
         before do
-          stub_licensed_features(security_dashboard: true)
-
           get :index, params: params, format: :json
         end
 
@@ -181,6 +179,7 @@ describe Projects::Security::DependenciesController do
     context 'with unauthorized user' do
       before do
         project.add_guest(user)
+        stub_licensed_features(dependency_list: true)
 
         get :index, params: params, format: :json
       end

ee/spec/serializers/dependency_entity_spec.rb

@@ -4,7 +4,14 @@ require 'spec_helper'
 
 describe DependencyEntity do
   describe '#as_json' do
-    let(:dependency) do
+    subject { described_class.represent(dependency, request: request).as_json }
+
+    set(:project) { create(:project, :repository, :private) }
+    set(:user) { create(:user) }
+    let(:request) { double('request') }
+    let(:dependency) { dependency_info.merge(vulnerabilities) }
+
+    let(:dependency_info) do
       {
         name:     'nokogiri',
         packager: 'Ruby (Bundler)',
@@ -12,7 +19,12 @@ describe DependencyEntity do
         location: {
           blob_path: '/some_project/path/Gemfile.lock',
           path:      'Gemfile.lock'
-        },
+        }
+      }
+    end
+
+    let(:vulnerabilities) do
+      {
         vulnerabilities:
           [{
              name:     'DDoS',
@@ -25,8 +37,28 @@ describe DependencyEntity do
       }
     end
 
-    subject { described_class.represent(dependency).as_json }
+    before do
+      stub_licensed_features(security_dashboard: true)
+      allow(request).to receive(:project).and_return(project)
+      allow(request).to receive(:user).and_return(user)
+    end
 
-    it { is_expected.to eq(dependency) }
+    context 'with developer' do
+      before do
+        project.add_developer(user)
+      end
+
+      it do
+        is_expected.to eq(dependency)
+      end
+    end
+
+    context 'with reporter' do
+      before do
+        project.add_reporter(user)
+      end
+
+      it { is_expected.to eq(dependency_info) }
+    end
   end
 end

ee/spec/serializers/dependency_list_entity_spec.rb

@@ -9,15 +9,19 @@ describe DependencyListEntity do
     end
 
     let(:request) { double('request') }
-    let(:project) { create(:project) }
+    set(:project) { create(:project, :repository, :private) }
+    set(:developer) { create(:user) }
 
     subject { entity.as_json }
 
     before do
+      project.add_developer(developer)
       allow(request).to receive(:project).and_return(project)
+      allow(request).to receive(:user).and_return(user)
     end
 
     context 'with success build' do
+      let(:user) { developer }
       let(:build) { create(:ee_ci_build, :success) }
 
       context 'with provided dependencies' do
@@ -43,6 +47,7 @@ describe DependencyListEntity do
       end
 
       context 'with no dependencies' do
+        let(:user) { developer }
         let(:dependencies) { [] }
 
         it 'has empty array of dependencies with status no_dependencies' do
@@ -59,19 +64,33 @@ describe DependencyListEntity do
       let(:build) { create(:ee_ci_build, :failed) }
       let(:dependencies) { [] }
 
+      context 'with authorized user' do
+        let(:user) { developer }
+
         it 'has job_path with status failed_job' do
           expect(subject[:report][:status]).to eq(:job_failed)
           expect(subject[:report]).to include(:job_path)
         end
       end
 
+      context 'without authorized user' do
+        let(:user) { create(:user) }
+
+        it 'has only status failed_job' do
+          expect(subject[:report][:status]).to eq(:job_failed)
+          expect(subject[:report]).not_to include(:job_path)
+        end
+      end
+    end
+
     context 'with no build' do
+      let(:user) { developer }
       let(:build) { nil }
       let(:dependencies) { [] }
 
       it 'has status job_not_set_up and no job_path' do
         expect(subject[:report][:status]).to eq(:job_not_set_up)
-        expect(subject[:report]).not_to include(:job_path)
+        expect(subject[:report][:job_path]).not_to be_present
       end
     end
   end

ee/spec/serializers/dependency_list_serializer_spec.rb

@@ -3,9 +3,13 @@
 require 'spec_helper'
 
 describe DependencyListSerializer do
-  let(:serializer) { described_class.new(project: project).represent(dependencies, build: build) }
   let(:build) { create(:ee_ci_build, :success) }
-  let(:project) { create(:project) }
+  set(:project) { create(:project, :repository, :private) }
+  set(:user) { create(:user) }
+
+  let(:serializer) do
+    described_class.new(project: project, user: user).represent(dependencies, build: build)
+  end
 
   let(:dependencies) do
     [{
@@ -24,6 +28,11 @@ describe DependencyListSerializer do
      }]
   end
 
+  before do
+    stub_licensed_features(security_dashboard: true)
+    project.add_developer(user)
+  end
+
   describe "#to_json" do
     subject { serializer.to_json }