Merge branch '212979-jira-import-permissions' into 'master'

Adjust permissions on jira import feature

See merge request gitlab-org/gitlab!29239

app/controllers/projects/import/jira_controller.rb

@@ -3,14 +3,18 @@
 module Projects
   module Import
     class JiraController < Projects::ApplicationController
+      before_action :authenticate_user!
+      before_action :check_issues_available!
+      before_action :authorize_read_project!
       before_action :jira_import_enabled?
       before_action :jira_integration_configured?
+      before_action :authorize_admin_project!, only: [:import]
 
       def show
         @is_jira_configured = @project.jira_service.present?
         return if Feature.enabled?(:jira_issue_import_vue, @project)
 
-        unless @project.latest_jira_import&.in_progress?
+        if !@project.latest_jira_import&.in_progress? && current_user&.can?(:admin_project, @project)
           jira_client = @project.jira_service.client
           jira_projects = jira_client.Project.all
 

app/graphql/resolvers/projects/jira_imports_resolver.rb

@@ -16,7 +16,7 @@ module Resolvers
       def authorized_resource?(project)
         return false unless project.jira_issues_import_feature_flag_enabled?
 
-        Ability.allowed?(context[:current_user], :admin_project, project)
+        context[:current_user].present? && Ability.allowed?(context[:current_user], :read_project, project)
       end
     end
   end

app/services/jira_import/start_import_service.rb

@@ -46,6 +46,7 @@ module JiraImport
     def validate
       return build_error_response(_('Jira import feature is disabled.')) unless project.jira_issues_import_feature_flag_enabled?
       return build_error_response(_('You do not have permissions to run the import.')) unless user.can?(:admin_project, project)
+      return build_error_response(_('Cannot import because issues are not available in this project.')) unless project.feature_available?(:issues, user)
       return build_error_response(_('Jira integration not configured.')) unless project.jira_service&.active?
       return build_error_response(_('Unable to find Jira project to import data from.')) if jira_project_key.blank?
       return build_error_response(_('Jira import is already running.')) if import_in_progress?

locale/gitlab.pot

@@ -3418,6 +3418,9 @@ msgstr ""
 msgid "Cannot have multiple Jira imports running at the same time"
 msgstr ""
 
+msgid "Cannot import because issues are not available in this project."
+msgstr ""
+
 msgid "Cannot make epic confidential if it contains not-confidential issues"
 msgstr ""
 

spec/controllers/projects/import/jira_controller_spec.rb

@@ -5,6 +5,7 @@ require 'spec_helper'
 describe Projects::Import::JiraController do
   let_it_be(:user) { create(:user) }
   let_it_be(:project) { create(:project) }
+  let_it_be(:jira_project_key) { 'Test' }
 
   context 'with anonymous user' do
     before do
@@ -21,7 +22,7 @@ describe Projects::Import::JiraController do
 
     context 'post import' do
       it 'redirects to issues page' do
-        post :import, params: { namespace_id: project.namespace, project_id: project, jira_project_key: 'Test' }
+        post :import, params: { namespace_id: project.namespace, project_id: project, jira_project_key: jira_project_key }
 
         expect(response).to redirect_to(new_user_session_path)
       end
@@ -49,7 +50,7 @@ describe Projects::Import::JiraController do
 
       context 'post import' do
         it 'redirects to issues page' do
-          post :import, params: { namespace_id: project.namespace, project_id: project, jira_project_key: 'Test' }
+          post :import, params: { namespace_id: project.namespace, project_id: project, jira_project_key: jira_project_key }
 
           expect(response).to redirect_to(project_issues_path(project))
         end
@@ -65,12 +66,64 @@ describe Projects::Import::JiraController do
       context 'when jira service is enabled for the project' do
         let_it_be(:jira_service) { create(:jira_service, project: project) }
 
+        context 'when user is developer' do
+          let_it_be(:dev) { create(:user) }
+
+          before do
+            sign_in(dev)
+            project.add_developer(dev)
+          end
+
+          context 'get show' do
+            before do
+              get :show, params: { namespace_id: project.namespace.to_param, project_id: project }
+            end
+
+            it 'does not query jira service' do
+              expect(project).not_to receive(:jira_service)
+            end
+
+            it 'renders show template' do
+              expect(response).to render_template(:show)
+              expect(assigns(:jira_projects)).not_to be_present
+            end
+          end
+
+          context 'post import' do
+            it 'returns 404' do
+              post :import, params: { namespace_id: project.namespace, project_id: project, jira_project_key: jira_project_key }
+
+              expect(response).to have_gitlab_http_status(:not_found)
+            end
+          end
+        end
+
+        context 'when issues disabled' do
+          let_it_be(:disabled_issues_project) { create(:project, :public, :issues_disabled) }
+
+          context 'get show' do
+            it 'returs 404' do
+              get :show, params: { namespace_id: project.namespace.to_param, project_id: disabled_issues_project }
+
+              expect(response).to have_gitlab_http_status(:not_found)
+            end
+          end
+
+          context 'post import' do
+            it 'returs 404' do
+              post :import, params: { namespace_id: disabled_issues_project.namespace, project_id: disabled_issues_project, jira_project_key: jira_project_key }
+
+              expect(response).to have_gitlab_http_status(:not_found)
+            end
+          end
+        end
+
         context 'when running jira import first time' do
           context 'get show' do
             before do
               allow(JIRA::Resource::Project).to receive(:all).and_return(jira_projects)
 
-              expect(project.import_state).to be_nil
+              expect(project.jira_imports).to be_empty
 
               get :show, params: { namespace_id: project.namespace.to_param, project_id: project }
             end
@@ -84,7 +137,7 @@ describe Projects::Import::JiraController do
               end
             end
 
-            context 'when everything is ok' do
+            context 'when projects retrieved from Jira' do
               let(:jira_projects) { [double(name: 'FOO project', key: 'FOO')] }
 
               it 'renders show template' do
@@ -107,14 +160,14 @@ describe Projects::Import::JiraController do
               it 'creates import state' do
                 expect(project.latest_jira_import).to be_nil
 
-                post :import, params: { namespace_id: project.namespace, project_id: project, jira_project_key: 'Test' }
+                post :import, params: { namespace_id: project.namespace, project_id: project, jira_project_key: jira_project_key }
 
                 project.reload
 
                 jira_import = project.latest_jira_import
                 expect(project.import_type).to eq 'jira'
                 expect(jira_import.status).to eq 'scheduled'
-                expect(jira_import.jira_project_key).to eq 'Test'
+                expect(jira_import.jira_project_key).to eq jira_project_key
                 expect(response).to redirect_to(project_import_jira_path(project))
               end
             end
@@ -145,7 +198,7 @@ describe Projects::Import::JiraController do
         end
 
         context 'when jira import ran before' do
-          let_it_be(:jira_import_state) { create(:jira_import_state, :finished, project: project, jira_project_key: 'Test') }
+          let_it_be(:jira_import_state) { create(:jira_import_state, :finished, project: project, jira_project_key: jira_project_key) }
 
           context 'get show' do
             it 'renders import status' do
@@ -164,7 +217,7 @@ describe Projects::Import::JiraController do
               project.reload
               expect(project.latest_jira_import.status).to eq 'scheduled'
               expect(project.jira_imports.size).to eq 2
-              expect(project.jira_imports.first.jira_project_key).to eq 'Test'
+              expect(project.jira_imports.first.jira_project_key).to eq jira_project_key
               expect(project.jira_imports.last.jira_project_key).to eq 'New Project'
               expect(response).to redirect_to(project_import_jira_path(project))
             end

spec/graphql/resolvers/projects/jira_imports_resolver_spec.rb

@@ -18,22 +18,20 @@ describe Resolvers::Projects::JiraImportsResolver do
 
           it_behaves_like 'no jira import access'
         end
-
-        context 'when user developer' do
-          before do
-            project.add_developer(user)
-          end
-
-          it_behaves_like 'no jira import access'
-        end
       end
 
       context 'when user can read Jira import data' do
         before do
-          project.add_maintainer(user)
+          project.add_guest(user)
         end
 
         it_behaves_like 'no jira import data present'
+
+        it 'does not raise access error' do
+          expect do
+            resolve_imports
+          end.not_to raise_error
+        end
       end
     end
 
@@ -58,19 +56,11 @@ describe Resolvers::Projects::JiraImportsResolver do
 
           it_behaves_like 'no jira import access'
         end
-
-        context 'when user developer' do
-          before do
-            project.add_developer(user)
-          end
-
-          it_behaves_like 'no jira import access'
-        end
       end
 
       context 'when user can access Jira imports' do
         before do
-          project.add_maintainer(user)
+          project.add_guest(user)
         end
 
         it 'returns Jira imports sorted ascending by created_at time' do

spec/requests/api/graphql/mutations/jira_import/start_spec.rb

@@ -99,6 +99,12 @@ describe 'Starting a Jira Import' do
           it_behaves_like 'a mutation that returns errors in the response', errors: ['Jira integration not configured.']
         end
 
+        context 'when issues feature are disabled' do
+          let_it_be(:project, reload: true) { create(:project, :issues_disabled) }
+
+          it_behaves_like 'a mutation that returns errors in the response', errors: ['Cannot import because issues are not available in this project.']
+        end
+
         context 'when when project has Jira service' do
           let!(:service) { create(:jira_service, project: project) }
 

spec/services/jira_import/start_import_service_spec.rb

@@ -38,6 +38,12 @@ describe JiraImport::StartImportService do
         it_behaves_like 'responds with error', 'Jira integration not configured.'
       end
 
+      context 'when issues feature are disabled' do
+        let_it_be(:project, reload: true) { create(:project, :issues_disabled) }
+
+        it_behaves_like 'responds with error', 'Cannot import because issues are not available in this project.'
+      end
+
       context 'when Jira service exists' do
         let!(:jira_service) { create(:jira_service, project: project, active: true) }