Merge branch 'security-fix-import-url-bypass' into 'master'

Ensure disabled import source is always respected

See merge request gitlab-org/security/gitlab!1751

app/controllers/projects_controller.rb

@@ -19,6 +19,7 @@ class ProjectsController < Projects::ApplicationController
   before_action :redirect_git_extension, only: [:show]
   before_action :project, except: [:index, :new, :create, :resolve]
   before_action :repository, except: [:index, :new, :create, :resolve]
+  before_action :verify_git_import_enabled, only: [:create]
   before_action :project_export_enabled, only: [:export, :download_export, :remove_export, :generate_new_export]
   before_action :present_project, only: [:edit]
   before_action :authorize_download_code!, only: [:refs]
@@ -495,6 +496,10 @@ class ProjectsController < Projects::ApplicationController
     url_for(safe_params)
   end
 
+  def verify_git_import_enabled
+    render_404 if project_params[:import_url] && !git_import_enabled?
+  end
+
   def project_export_enabled
     render_404 unless Gitlab::CurrentSettings.project_export_enabled?
   end

lib/api/projects.rb

@@ -89,6 +89,10 @@ module API
           Gitlab::AppLogger.info({ message: "File exceeds maximum size", file_bytes: file.size, project_id: user_project.id, project_path: user_project.full_path, upload_allowed: allowed })
         end
       end
+
+      def check_import_by_url_is_enabled
+        forbidden! unless Gitlab::CurrentSettings.import_sources&.include?('git')
+      end
     end
 
     helpers do
@@ -267,6 +271,7 @@ module API
         attrs = declared_params(include_missing: false)
         attrs = translate_params_for_compatibility(attrs)
         filter_attributes_using_license!(attrs)
+        check_import_by_url_is_enabled if params[:import_url].present?
         project = ::Projects::CreateService.new(current_user, attrs).execute
 
         if project.saved?

spec/controllers/projects_controller_spec.rb

@@ -419,6 +419,47 @@ RSpec.describe ProjectsController do
     end
   end
 
+  describe 'POST create' do
+    let!(:params) do
+      {
+        path: 'foo',
+        description: 'bar',
+        import_url: project.http_url_to_repo,
+        namespace_id: user.namespace.id
+      }
+    end
+
+    subject { post :create, params: { project: params } }
+
+    before do
+      sign_in(user)
+    end
+
+    context 'when import by url is disabled' do
+      before do
+        stub_application_setting(import_sources: [])
+      end
+
+      it 'does not create project and reports an error' do
+        expect { subject }.not_to change { Project.count }
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+
+    context 'when import by url is enabled' do
+      before do
+        stub_application_setting(import_sources: ['git'])
+      end
+
+      it 'creates project' do
+        expect { subject }.to change { Project.count }
+
+        expect(response).to have_gitlab_http_status(:redirect)
+      end
+    end
+  end
+
   describe 'GET edit' do
     it 'allows an admin user to access the page', :enable_admin_mode do
       sign_in(create(:user, :admin))

spec/requests/api/projects_spec.rb

@@ -1149,6 +1149,16 @@ RSpec.describe API::Projects do
       expect(response).to have_gitlab_http_status(:bad_request)
     end
 
+    it 'disallows creating a project with an import_url when git import source is disabled' do
+      stub_application_setting(import_sources: nil)
+
+      project_params = { import_url: 'http://example.com', path: 'path-project-Foo', name: 'Foo Project' }
+      expect { post api('/projects', user), params: project_params }
+        .not_to change {  Project.count }
+
+      expect(response).to have_gitlab_http_status(:forbidden)
+    end
+
     it 'sets a project as public' do
       project = attributes_for(:project, visibility: 'public')