Merge branch '37791-merge-dependency-controllers' into 'master'

Merge dependencies controllers together

See merge request gitlab-org/gitlab!21885

ee/app/assets/javascripts/dependencies/store/modules/list/getters.js

@@ -9,4 +9,4 @@ export const isJobFailed = state =>
   [REPORT_STATUS.jobFailed, REPORT_STATUS.noDependencies].includes(state.reportInfo.status);
 export const isIncomplete = state => state.reportInfo.status === REPORT_STATUS.incomplete;
 
-export const downloadEndpoint = ({ endpoint }) => `${endpoint}.json`;
+export const downloadEndpoint = ({ endpoint }) => endpoint;

ee/app/controllers/projects/dependencies_controller.rb

@@ -4,8 +4,83 @@ module Projects
   class DependenciesController < Projects::ApplicationController
     before_action :authorize_read_dependency_list!
 
+    def index
+      respond_to do |format|
+        format.html do
+          render status: :ok
+        end
+        format.json do
+          ::Gitlab::UsageCounters::DependencyList.increment(project.id)
+
+          render json: serializer.represent(dependencies, build: report_service.build)
+        end
+      end
+    end
+
+    private
+
+    def can_access_vulnerable?
+      return true unless query_params[:filter] == 'vulnerable'
+
+      can?(current_user, :read_vulnerability, project)
+    end
+
+    def can_collect_dependencies?
+      report_service.able_to_fetch? && can_access_vulnerable?
+    end
+
+    def collect_dependencies
+      found_dependencies = can_collect_dependencies? ? service.execute : []
+      ::Gitlab::ItemsCollection.new(found_dependencies)
+    end
+
     def authorize_read_dependency_list!
-      render_404 unless can?(current_user, :read_dependencies, project)
+      return if can?(current_user, :read_dependencies, project)
+
+      respond_to do |format|
+        format.html do
+          render_404
+        end
+        format.json do
+          render_403
+        end
+      end
+    end
+
+    def dependencies
+      @dependencies ||= collect_dependencies
+    end
+
+    def match_disallowed(param, value)
+      param == :sort_by && !value.in?(::Security::DependencyListService::SORT_BY_VALUES) ||
+        param == :sort && !value.in?(::Security::DependencyListService::SORT_VALUES) ||
+        param == :filter && !value.in?(::Security::DependencyListService::FILTER_VALUES)
+    end
+
+    def pipeline
+      @pipeline ||= report_service.pipeline
+    end
+
+    def query_params
+      return @permitted_params if @permitted_params
+
+      @permitted_params = params.permit(:sort, :sort_by, :filter).delete_if do |key, value|
+        match_disallowed(key, value)
+      end
+    end
+
+    def report_service
+      @report_service ||= ::Security::ReportFetchService.new(project, ::Ci::JobArtifact.dependency_list_reports)
+    end
+
+    def serializer
+      serializer = ::DependencyListSerializer.new(project: project, user: current_user)
+      serializer = serializer.with_pagination(request, response) if params[:page]
+      serializer
+    end
+
+    def service
+      ::Security::DependencyListService.new(pipeline: pipeline, params: query_params)
     end
   end
 end

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

-# frozen_string_literal: true
-
-module Projects
-  module Security
-    class DependenciesController < Projects::ApplicationController
-      before_action :authorize_read_dependency_list!
-
-      def index
-        respond_to do |format|
-          format.json do
-            ::Gitlab::UsageCounters::DependencyList.increment(project.id)
-
-            render json: serializer.represent(dependencies, build: report_service.build)
-          end
-        end
-      end
-
-      private
-
-      def can_access_vulnerable?
-        return true unless query_params[:filter] == 'vulnerable'
-
-        can?(current_user, :read_vulnerability, project)
-      end
-
-      def can_collect_dependencies?
-        report_service.able_to_fetch? && can_access_vulnerable?
-      end
-
-      def collect_dependencies
-        found_dependencies = can_collect_dependencies? ? service.execute : []
-        ::Gitlab::ItemsCollection.new(found_dependencies)
-      end
-
-      def authorize_read_dependency_list!
-        render_403 unless can?(current_user, :read_dependencies, project)
-      end
-
-      def dependencies
-        @dependencies ||= collect_dependencies
-      end
-
-      def match_disallowed(param, value)
-        param == :sort_by && !value.in?(::Security::DependencyListService::SORT_BY_VALUES) ||
-          param == :sort && !value.in?(::Security::DependencyListService::SORT_VALUES) ||
-          param == :filter && !value.in?(::Security::DependencyListService::FILTER_VALUES)
-      end
-
-      def pipeline
-        @pipeline ||= report_service.pipeline
-      end
-
-      def query_params
-        return @permitted_params if @permitted_params
-
-        @permitted_params = params.permit(:sort, :sort_by, :filter).delete_if do |key, value|
-          match_disallowed(key, value)
-        end
-      end
-
-      def report_service
-        @report_service ||= ::Security::ReportFetchService.new(project, ::Ci::JobArtifact.dependency_list_reports)
-      end
-
-      def serializer
-        serializer = ::DependencyListSerializer.new(project: project, user: current_user)
-        serializer = serializer.with_pagination(request, response) if params[:page]
-        serializer
-      end
-
-      def service
-        ::Security::DependencyListService.new(pipeline: pipeline, params: query_params)
-      end
-    end
-  end
-end

ee/app/views/layouts/nav/sidebar/_project_security_link.html.haml

@@ -24,7 +24,7 @@
           %span= _('Security Dashboard')
 
     - if project_nav_tab?(:dependencies)
-      = nav_link(path: 'projects/dependencies#show') do
+      = nav_link(path: 'projects/dependencies#index') do
         = link_to project_dependencies_path(@project), title: _('Dependency List'), data: { qa_selector: 'dependency_list_link' } do
           %span= _('Dependency List')
 

ee/app/views/projects/dependencies/index.html.haml

+- breadcrumb_title _('Dependency List')
+- page_title _('Dependency List')
+
+#js-dependencies-app{ data: { endpoint: project_dependencies_path(@project, format: :json), documentation_path: help_page_path('user/application_security/dependency_list/index'), empty_state_svg_path: image_path('illustrations/Dependency-list-empty-state.svg') } }

ee/app/views/projects/dependencies/show.html.haml

-- breadcrumb_title _('Dependency List')
-- page_title _('Dependency List')
-
-#js-dependencies-app{ data: { endpoint: project_security_dependencies_path(@project), documentation_path: help_page_path('user/application_security/dependency_list/index'), empty_state_svg_path: image_path('illustrations/Dependency-list-empty-state.svg') } }

ee/config/routes/project.rb

@@ -191,7 +191,6 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
         resource :dashboard, only: [:show], controller: :dashboard
         resource :configuration, only: [:show], controller: :configuration
 
-        resources :dependencies, only: [:index]
         # We have to define both legacy and new routes for Vulnerability Findings
         # because they are loaded upon application initialization and preloaded by
         # web server.
@@ -211,7 +210,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
 
       resources :vulnerability_feedback, only: [:index, :create, :update, :destroy], constraints: { id: /\d+/ }
 
-      resource :dependencies, only: [:show]
+      resources :dependencies, only: [:index]
       # All new routes should go under /-/ scope.
       # Look for scope '-' at the top of the file.
       # rubocop: enable Cop/PutProjectRoutesUnderScope

ee/spec/controllers/projects/dependencies_controller_spec.rb

@@ -3,74 +3,264 @@
 require 'spec_helper'
 
 describe Projects::DependenciesController do
-  set(:project) { create(:project, :repository, :public, :repository_private) }
-  set(:user) { create(:user) }
+  describe 'GET #index' do
+    let_it_be(:developer) { create(:user) }
+    let_it_be(:guest) { create(:user) }
+    let(:params) { { namespace_id: project.namespace, project_id: project } }
 
-  subject { get :show, params: { namespace_id: project.namespace, project_id: project } }
+    before do
+      sign_in(user)
+    end
 
-  describe 'GET show' do
     context 'with authorized user' do
+      let_it_be(:project) { create(:project, :repository, :public) }
+
       before do
-        project.add_reporter(user)
-        sign_in(user)
+        project.add_developer(developer)
+        project.add_guest(guest)
       end
 
       context 'when feature is available' do
-        render_views
-
         before do
-          stub_licensed_features(dependency_scanning: true)
+          stub_licensed_features(dependency_scanning: true, license_management: true, security_dashboard: true)
         end
 
-        it 'renders the show template' do
-          subject
+        context 'when requesting HTML' do
+          render_views
+          let(:user) { developer }
+
+          before do
+            get :index, params: params, format: :html
+          end
 
-          expect(response).to have_gitlab_http_status(200)
-          expect(response).to render_template(:show)
+          it { expect(response).to have_http_status(:ok) }
+
+          it 'renders the side navigation with the correct submenu set as active' do
+            expect(response.body).to have_active_sub_navigation('Dependency List')
+          end
         end
 
-        it 'renders the side navigation with the correct submenu set as active' do
-          subject
+        context 'when usage ping is collected' do
+          let(:user) { developer }
+
+          it 'counts usage of the feature' do
+            expect(::Gitlab::UsageCounters::DependencyList).to receive(:increment).with(project.id)
 
-          expect(response.body).to have_active_sub_navigation('Dependency List')
+            get :index, params: params, format: :json
+          end
         end
-      end
 
-      context 'when feature is not available' do
-        it 'returns 404' do
-          subject
+        context 'with existing report' do
+          let!(:pipeline) { create(:ee_ci_pipeline, :with_dependency_list_report, project: project) }
 
-          expect(response).to have_gitlab_http_status(404)
+          before do
+            get :index, params: params, format: :json
+          end
+
+          context 'without pagination params' do
+            let(:user) { developer }
+
+            it 'returns a hash with dependencies' do
+              expect(json_response).to be_a(Hash)
+              expect(json_response['dependencies'].length).to eq(21)
+            end
+
+            it 'returns status ok' do
+              expect(json_response['report']['status']).to eq('ok')
+            end
+
+            it 'returns job path' do
+              job_path = "/#{project.full_path}/builds/#{pipeline.builds.last.id}"
+
+              expect(json_response['report']['job_path']).to eq(job_path)
+            end
+
+            it 'returns success code' do
+              expect(response).to have_gitlab_http_status(200)
+            end
+          end
+
+          context 'with params' do
+            context 'with sorting params' do
+              let(:user) { developer }
+
+              context 'when sorted by packager' do
+                let(:params) do
+                  {
+                    namespace_id: project.namespace,
+                    project_id: project,
+                    sort_by: 'packager',
+                    sort: 'desc',
+                    page: 1
+                  }
+                end
+
+                it 'returns sorted list' do
+                  expect(json_response['dependencies'].first['packager']).to eq('Ruby (Bundler)')
+                  expect(json_response['dependencies'].last['packager']).to eq('JavaScript (Yarn)')
+                end
+
+                it 'return 20 dependencies' do
+                  expect(json_response['dependencies'].length).to eq(20)
+                end
+              end
+
+              context 'when sorted by severity' do
+                let(:params) do
+                  {
+                    namespace_id: project.namespace,
+                    project_id: project,
+                    sort_by: 'severity',
+                    page: 1
+                  }
+                end
+
+                it 'returns sorted list' do
+                  expect(json_response['dependencies'].first['name']).to eq('nokogiri')
+                  expect(json_response['dependencies'].second['name']).to eq('debug')
+                end
+              end
+            end
+
+            context 'with filter by vulnerable' do
+              let(:params) do
+                {
+                  namespace_id: project.namespace,
+                  project_id: project,
+                  filter: 'vulnerable'
+                }
+              end
+
+              context 'with authorized user to see vulnerabilities' do
+                let(:user) { developer }
+
+                it 'return vulnerable dependencies' do
+                  expect(json_response['dependencies'].length).to eq(3)
+                end
+              end
+
+              context 'without authorized user to see vulnerabilities' do
+                let(:user) { guest }
+
+                it 'return vulnerable dependencies' do
+                  expect(json_response['dependencies']).to be_empty
+                end
+              end
+            end
+
+            context 'with pagination params' do
+              let(:user) { developer }
+              let(:params) { { namespace_id: project.namespace, project_id: project, page: 2 } }
+
+              it 'returns paginated list' do
+                expect(json_response['dependencies'].length).to eq(1)
+                expect(response).to include_pagination_headers
+              end
+            end
+          end
         end
-      end
-    end
 
-    context 'with unauthorized user' do
-      before do
-        sign_in(user)
+        context 'with found license report' do
+          let(:user) { developer }
+          let(:pipeline) { create(:ee_ci_pipeline, :with_dependency_list_report, project: project) }
+          let(:license_build) { create(:ee_ci_build, :success, :license_management, pipeline: pipeline) }
+
+          before do
+            pipeline.builds << license_build
+
+            get :index, params: params, format: :json
+          end
+
+          it 'include license information to response' do
+            nokogiri = json_response['dependencies'].select { |dep| dep['name'] == 'nokogiri' }.first
+
+            expect(nokogiri['licenses']).not_to be_empty
+          end
+        end
+
+        context 'with a report of the wrong type' do
+          let(:user) { developer }
+          let!(:pipeline) { create(:ee_ci_pipeline, :with_sast_report, project: project) }
+
+          before do
+            get :index, params: params, format: :json
+          end
+
+          it 'returns job_not_set_up status' do
+            expect(json_response['report']['status']).to eq('job_not_set_up')
+          end
+
+          it 'returns a nil job_path' do
+            expect(json_response['report']['job_path']).to be_nil
+          end
+        end
+
+        context 'when report doesn\'t have dependency list' do
+          let(:user) { developer }
+          let!(:pipeline) { create(:ee_ci_pipeline, :with_dependency_scanning_report, project: project) }
+
+          before do
+            get :index, params: params, format: :json
+          end
+
+          it 'returns no_dependencies status' do
+            expect(json_response['report']['status']).to eq('no_dependencies')
+          end
+        end
+
+        context 'when job failed' do
+          let(:user) { developer }
+          let!(:pipeline) { create(:ee_ci_pipeline, :success, project: project) }
+          let!(:build) { create(:ee_ci_build, :dependency_list, :failed, :allowed_to_fail) }
+
+          before do
+            pipeline.builds << build
+
+            get :index, params: params, format: :json
+          end
+
+          it 'returns job_failed status' do
+            expect(json_response['report']['status']).to eq('job_failed')
+          end
+        end
       end
 
-      context 'when feature is available' do
-        before do
-          stub_licensed_features(dependency_scanning: true)
+      context 'when licensed feature is unavailable' do
+        let(:user) { developer }
+
+        it 'returns 403 for a JSON request' do
+          get :index, params: params, format: :json
+
+          expect(response).to have_gitlab_http_status(403)
         end
 
-        it 'returns 404' do
-          subject
+        it 'returns a 404 for an HTML request' do
+          get :index, params: params, format: :html
 
           expect(response).to have_gitlab_http_status(404)
         end
       end
     end
 
-    context 'with anonymous user and private project' do
+    context 'with unauthorized user' do
       let(:project) { create(:project, :repository, :private) }
+      let(:user) { guest }
+
+      before do
+        stub_licensed_features(dependency_scanning: true)
+        project.add_guest(user)
+      end
+
+      it 'returns 403 for a JSON request' do
+        get :index, params: params, format: :json
+
+        expect(response).to have_gitlab_http_status(403)
+      end
 
-      it 'returns 302' do
-        subject
+      it 'returns a 404 for an HTML request' do
+        get :index, params: params, format: :html
 
-        expect(response).to have_gitlab_http_status(302)
-        expect(response).to redirect_to(new_user_session_path)
+        expect(response).to have_gitlab_http_status(404)
       end
     end
   end

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

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe Projects::Security::DependenciesController do
-  describe 'GET index.json' do
-    let_it_be(:developer) { create(:user) }
-    let_it_be(:guest) { create(:user) }
-    let(:params) { { namespace_id: project.namespace, project_id: project } }
-
-    before do
-      sign_in(user)
-    end
-
-    context 'with authorized user' do
-      let_it_be(:project) { create(:project, :repository, :public) }
-
-      before do
-        project.add_developer(developer)
-        project.add_guest(guest)
-      end
-
-      context 'when feature is available' do
-        before do
-          stub_licensed_features(dependency_scanning: true, license_management: true, security_dashboard: true)
-        end
-
-        context 'when usage ping is collected' do
-          let(:user) { developer }
-
-          it 'counts usage of the feature' do
-            expect(::Gitlab::UsageCounters::DependencyList).to receive(:increment).with(project.id)
-
-            get :index, params: params, format: :json
-          end
-        end
-
-        context 'with existing report' do
-          let!(:pipeline) { create(:ee_ci_pipeline, :with_dependency_list_report, project: project) }
-
-          before do
-            get :index, params: params, format: :json
-          end
-
-          context 'without pagination params' do
-            let(:user) { developer }
-
-            it 'returns a hash with dependencies' do
-              expect(json_response).to be_a(Hash)
-              expect(json_response['dependencies'].length).to eq(21)
-            end
-
-            it 'returns status ok' do
-              expect(json_response['report']['status']).to eq('ok')
-            end
-
-            it 'returns job path' do
-              job_path = "/#{project.full_path}/builds/#{pipeline.builds.last.id}"
-
-              expect(json_response['report']['job_path']).to eq(job_path)
-            end
-
-            it 'returns success code' do
-              expect(response).to have_gitlab_http_status(200)
-            end
-          end
-
-          context 'with params' do
-            context 'with sorting params' do
-              let(:user) { developer }
-
-              context 'when sorted by packager' do
-                let(:params) do
-                  {
-                    namespace_id: project.namespace,
-                    project_id: project,
-                    sort_by: 'packager',
-                    sort: 'desc',
-                    page: 1
-                  }
-                end
-
-                it 'returns sorted list' do
-                  expect(json_response['dependencies'].first['packager']).to eq('Ruby (Bundler)')
-                  expect(json_response['dependencies'].last['packager']).to eq('JavaScript (Yarn)')
-                end
-
-                it 'return 20 dependencies' do
-                  expect(json_response['dependencies'].length).to eq(20)
-                end
-              end
-
-              context 'when sorted by severity' do
-                let(:params) do
-                  {
-                    namespace_id: project.namespace,
-                    project_id: project,
-                    sort_by: 'severity',
-                    page: 1
-                  }
-                end
-
-                it 'returns sorted list' do
-                  expect(json_response['dependencies'].first['name']).to eq('nokogiri')
-                  expect(json_response['dependencies'].second['name']).to eq('debug')
-                end
-              end
-            end
-
-            context 'with filter by vulnerable' do
-              let(:params) do
-                {
-                  namespace_id: project.namespace,
-                  project_id: project,
-                  filter: 'vulnerable'
-                }
-              end
-
-              context 'with authorized user to see vulnerabilities' do
-                let(:user) { developer }
-
-                it 'return vulnerable dependencies' do
-                  expect(json_response['dependencies'].length).to eq(3)
-                end
-              end
-
-              context 'without authorized user to see vulnerabilities' do
-                let(:user) { guest }
-
-                it 'return vulnerable dependencies' do
-                  expect(json_response['dependencies']).to be_empty
-                end
-              end
-            end
-
-            context 'with pagination params' do
-              let(:user) { developer }
-              let(:params) { { namespace_id: project.namespace, project_id: project, page: 2 } }
-
-              it 'returns paginated list' do
-                expect(json_response['dependencies'].length).to eq(1)
-                expect(response).to include_pagination_headers
-              end
-            end
-          end
-        end
-
-        context 'with found license report' do
-          let(:user) { developer }
-          let(:pipeline) { create(:ee_ci_pipeline, :with_dependency_list_report, project: project) }
-          let(:license_build) { create(:ee_ci_build, :success, :license_management, pipeline: pipeline) }
-
-          before do
-            pipeline.builds << license_build
-
-            get :index, params: params, format: :json
-          end
-
-          it 'include license information to response' do
-            nokogiri = json_response['dependencies'].select { |dep| dep['name'] == 'nokogiri' }.first
-
-            expect(nokogiri['licenses']).not_to be_empty
-          end
-        end
-
-        context 'with a report of the wrong type' do
-          let(:user) { developer }
-          let!(:pipeline) { create(:ee_ci_pipeline, :with_sast_report, project: project) }
-
-          before do
-            get :index, params: params, format: :json
-          end
-
-          it 'returns job_not_set_up status' do
-            expect(json_response['report']['status']).to eq('job_not_set_up')
-          end
-
-          it 'returns a nil job_path' do
-            expect(json_response['report']['job_path']).to be_nil
-          end
-        end
-
-        context 'when report doesn\'t have dependency list' do
-          let(:user) { developer }
-          let!(:pipeline) { create(:ee_ci_pipeline, :with_dependency_scanning_report, project: project) }
-
-          before do
-            get :index, params: params, format: :json
-          end
-
-          it 'returns no_dependencies status' do
-            expect(json_response['report']['status']).to eq('no_dependencies')
-          end
-        end
-
-        context 'when job failed' do
-          let(:user) { developer }
-          let!(:pipeline) { create(:ee_ci_pipeline, :success, project: project) }
-          let!(:build) { create(:ee_ci_build, :dependency_list, :failed, :allowed_to_fail) }
-
-          before do
-            pipeline.builds << build
-
-            get :index, params: params, format: :json
-          end
-
-          it 'returns job_failed status' do
-            expect(json_response['report']['status']).to eq('job_failed')
-          end
-        end
-      end
-
-      context 'when licensed feature is unavailable' do
-        let(:user) { developer }
-
-        before do
-          get :index, params: params, format: :json
-        end
-
-        it 'returns 403' do
-          expect(response).to have_gitlab_http_status(403)
-        end
-      end
-    end
-
-    context 'with unauthorized user' do
-      let(:project) { create(:project, :repository, :private) }
-      let(:user) { guest }
-
-      before do
-        stub_licensed_features(dependency_scanning: true)
-        project.add_guest(user)
-
-        get :index, params: params, format: :json
-      end
-
-      it 'returns 403' do
-        expect(response).to have_gitlab_http_status(403)
-      end
-    end
-  end
-end

ee/spec/frontend/dependencies/components/dependencies_actions_spec.js

@@ -31,7 +31,7 @@ describe('DependenciesActions component', () => {
     factory({
       propsData: { namespace },
     });
-    store.state[namespace].endpoint = `${TEST_HOST}/dependencies`;
+    store.state[namespace].endpoint = `${TEST_HOST}/dependencies.json`;
     return wrapper.vm.$nextTick();
   });
 

ee/spec/frontend/dependencies/store/modules/list/getters_spec.js

@@ -28,7 +28,7 @@ describe('Dependencies getters', () => {
   describe('downloadEndpoint', () => {
     it('should return download endpoint', () => {
       const endpoint = `${TEST_HOST}/dependencies`;
-      expect(getters.downloadEndpoint({ endpoint })).toBe(`${TEST_HOST}/dependencies.json`);
+      expect(getters.downloadEndpoint({ endpoint })).toBe(endpoint);
     });
   });