Don't create groups for unallowed users when importing projects

12 files changed, 137 insertions, 56 deletions

diff --git a/app/controllers/import/base_controller.rb b/app/controllers/import/base_controller.rb
index 7e8597a5eb3..1ca33bc5d22 100644
--- a/app/controllers/import/base_controller.rb
+++ b/app/controllers/import/base_controller.rb
@@ -1,12 +1,16 @@
 class Import::BaseController < ApplicationController
   private
 
-  def get_or_create_namespace
+  def find_or_create_namespace(name, owner)
     begin
+      @target_namespace = params[:new_namespace].presence || name
+      @target_namespace = current_user.namespace_path if name == owner || !current_user.can_create_group?
+
       namespace = Group.create!(name: @target_namespace, path: @target_namespace, owner: current_user)
       namespace.add_owner(current_user)
     rescue ActiveRecord::RecordNotUnique, ActiveRecord::RecordInvalid
       namespace = Namespace.find_by_path_or_name(@target_namespace)
+
       unless current_user.can?(:create_projects, namespace)
         @already_been_taken = true
         return false
diff --git a/app/controllers/import/bitbucket_controller.rb b/app/controllers/import/bitbucket_controller.rb
index 944c73d139a..94e213b8743 100644
--- a/app/controllers/import/bitbucket_controller.rb
+++ b/app/controllers/import/bitbucket_controller.rb
@@ -35,15 +35,10 @@ class Import::BitbucketController < Import::BaseController
   end
 
   def create
-    @repo_id = params[:repo_id] || ""
-    repo = client.project(@repo_id.gsub("___", "/"))
-    @project_name = repo["slug"]
-
-    repo_owner = repo["owner"]
-    repo_owner = current_user.username if repo_owner == client.user["user"]["username"]
-    @target_namespace = params[:new_namespace].presence || repo_owner
-
-    namespace = get_or_create_namespace || (render and return)
+    @repo_id = params[:repo_id].to_s
+    repo = client.project(@repo_id.gsub('___', '/'))
+    @project_name = repo['slug']
+    namespace = find_or_create_namespace(repo['owner'], client.user['user']['username']) || (render and return)
 
     unless Gitlab::BitbucketImport::KeyAdder.new(repo, current_user, access_params).execute
       @access_denied = true
diff --git a/app/controllers/import/github_controller.rb b/app/controllers/import/github_controller.rb
index 9c1b0eb20f4..4047e62efa2 100644
--- a/app/controllers/import/github_controller.rb
+++ b/app/controllers/import/github_controller.rb
@@ -41,12 +41,7 @@ class Import::GithubController < Import::BaseController
     @repo_id = params[:repo_id].to_i
     repo = client.repo(@repo_id)
     @project_name = repo.name
-
-    repo_owner = repo.owner.login
-    repo_owner = current_user.username if repo_owner == client.user.login
-    @target_namespace = params[:new_namespace].presence || repo_owner
-
-    namespace = get_or_create_namespace || (render and return)
+    namespace = find_or_create_namespace(repo.owner.login, client.user.login) || (render and return)
 
     @project = Gitlab::GithubImport::ProjectCreator.new(repo, namespace, current_user, access_params).execute
   end
diff --git a/app/controllers/import/gitlab_controller.rb b/app/controllers/import/gitlab_controller.rb
index 08130ee8176..bc967e55ab1 100644
--- a/app/controllers/import/gitlab_controller.rb
+++ b/app/controllers/import/gitlab_controller.rb
@@ -26,13 +26,8 @@ class Import::GitlabController < Import::BaseController
   def create
     @repo_id = params[:repo_id].to_i
     repo = client.project(@repo_id)
-    @project_name = repo["name"]
-
-    repo_owner = repo["namespace"]["path"]
-    repo_owner = current_user.username if repo_owner == client.user["username"]
-    @target_namespace = params[:new_namespace].presence || repo_owner
-
-    namespace = get_or_create_namespace || (render and return)
+    @project_name = repo['name']
+    namespace = find_or_create_namespace(repo['namespace']['path'], client.user['username']) || (render and return)
 
     @project = Gitlab::GitlabImport::ProjectCreator.new(repo, namespace, current_user, access_params).execute
   end
diff --git a/app/helpers/import_helper.rb b/app/helpers/import_helper.rb
index 109bc1a02d1..021d2b14718 100644
--- a/app/helpers/import_helper.rb
+++ b/app/helpers/import_helper.rb
@@ -1,4 +1,9 @@
 module ImportHelper
+  def import_project_target(owner, name)
+    namespace = current_user.can_create_group? ? owner : current_user.namespace_path
+    "#{namespace}/#{name}"
+  end
+
   def github_project_link(path_with_namespace)
     link_to path_with_namespace, github_project_url(path_with_namespace), target: '_blank'
   end
diff --git a/app/views/import/bitbucket/status.html.haml b/app/views/import/bitbucket/status.html.haml
index 15dd98077c8..f8b4b107513 100644
--- a/app/views/import/bitbucket/status.html.haml
+++ b/app/views/import/bitbucket/status.html.haml
@@ -51,7 +51,7 @@
           %td
             = link_to "#{repo["owner"]}/#{repo["slug"]}", "https://bitbucket.org/#{repo["owner"]}/#{repo["slug"]}", target: "_blank"
           %td.import-target
-            = "#{repo["owner"]}/#{repo["slug"]}"
+            = import_project_target(repo['owner'], repo['slug'])
           %td.import-actions.job-status
             = button_tag class: "btn btn-import js-add-to-import" do
               Import
diff --git a/app/views/import/github/status.html.haml b/app/views/import/github/status.html.haml
index 54ff1d27c67..bd3be20c4f8 100644
--- a/app/views/import/github/status.html.haml
+++ b/app/views/import/github/status.html.haml
@@ -45,7 +45,7 @@
           %td
             = github_project_link(repo.full_name)
           %td.import-target
-            = repo.full_name
+            = import_project_target(repo.owner.login, repo.name)
           %td.import-actions.job-status
             = button_tag class: "btn btn-import js-add-to-import" do
               Import
diff --git a/app/views/import/gitlab/status.html.haml b/app/views/import/gitlab/status.html.haml
index fcfc6fd37f4..d31fc2e6adb 100644
--- a/app/views/import/gitlab/status.html.haml
+++ b/app/views/import/gitlab/status.html.haml
@@ -45,7 +45,7 @@
           %td
             = link_to repo["path_with_namespace"], "https://gitlab.com/#{repo["path_with_namespace"]}", target: "_blank"
           %td.import-target
-            = repo["path_with_namespace"]
+            = import_project_target(repo['namespace']['path'], repo['name'])
           %td.import-actions.job-status
             = button_tag class: "btn btn-import js-add-to-import" do
               Import
diff --git a/spec/controllers/import/bitbucket_controller_spec.rb b/spec/controllers/import/bitbucket_controller_spec.rb
index 07bf8d2d1c3..1d3c9fbbe2f 100644
--- a/spec/controllers/import/bitbucket_controller_spec.rb
+++ b/spec/controllers/import/bitbucket_controller_spec.rb
@@ -146,21 +146,42 @@ describe Import::BitbucketController do
       end
 
       context "when a namespace with the Bitbucket user's username doesn't exist" do
-        it "creates the namespace" do
-          expect(Gitlab::BitbucketImport::ProjectCreator).
-            to receive(:new).and_return(double(execute: true))
+        context "when current user can create namespaces" do
+          it "creates the namespace" do
+            expect(Gitlab::BitbucketImport::ProjectCreator).
+              to receive(:new).and_return(double(execute: true))
 
-          post :create, format: :js
+            expect { post :create, format: :js }.to change(Namespace, :count).by(1)
+          end
+
+          it "takes the new namespace" do
+            expect(Gitlab::BitbucketImport::ProjectCreator).
+              to receive(:new).with(bitbucket_repo, an_instance_of(Group), user, access_params).
+              and_return(double(execute: true))
 
-          expect(Namespace.where(name: other_username).first).not_to be_nil
+            post :create, format: :js
+          end
         end
 
-        it "takes the new namespace" do
-          expect(Gitlab::BitbucketImport::ProjectCreator).
-            to receive(:new).with(bitbucket_repo, an_instance_of(Group), user, access_params).
-            and_return(double(execute: true))
+        context "when current user can't create namespaces" do
+          before do
+            user.update_attribute(:can_create_group, false)
+          end
 
-          post :create, format: :js
+          it "doesn't create the namespace" do
+            expect(Gitlab::BitbucketImport::ProjectCreator).
+              to receive(:new).and_return(double(execute: true))
+
+            expect { post :create, format: :js }.not_to change(Namespace, :count)
+          end
+
+          it "takes the current user's namespace" do
+            expect(Gitlab::BitbucketImport::ProjectCreator).
+              to receive(:new).with(bitbucket_repo, user.namespace, user, access_params).
+              and_return(double(execute: true))
+
+            post :create, format: :js
+          end
         end
       end
     end
diff --git a/spec/controllers/import/github_controller_spec.rb b/spec/controllers/import/github_controller_spec.rb
index 51d59526854..ebfbf54182b 100644
--- a/spec/controllers/import/github_controller_spec.rb
+++ b/spec/controllers/import/github_controller_spec.rb
@@ -181,21 +181,42 @@ describe Import::GithubController do
       end
 
       context "when a namespace with the GitHub user's username doesn't exist" do
-        it "creates the namespace" do
-          expect(Gitlab::GithubImport::ProjectCreator).
-            to receive(:new).and_return(double(execute: true))
+        context "when current user can create namespaces" do
+          it "creates the namespace" do
+            expect(Gitlab::GithubImport::ProjectCreator).
+              to receive(:new).and_return(double(execute: true))
 
-          post :create, format: :js
+            expect { post :create, format: :js }.to change(Namespace, :count).by(1)
+          end
+
+          it "takes the new namespace" do
+            expect(Gitlab::GithubImport::ProjectCreator).
+              to receive(:new).with(github_repo, an_instance_of(Group), user, access_params).
+              and_return(double(execute: true))
 
-          expect(Namespace.where(name: other_username).first).not_to be_nil
+            post :create, format: :js
+          end
         end
 
-        it "takes the new namespace" do
-          expect(Gitlab::GithubImport::ProjectCreator).
-            to receive(:new).with(github_repo, an_instance_of(Group), user, access_params).
-            and_return(double(execute: true))
+        context "when current user can't create namespaces" do
+          before do
+            user.update_attribute(:can_create_group, false)
+          end
 
-          post :create, format: :js
+          it "doesn't create the namespace" do
+            expect(Gitlab::GithubImport::ProjectCreator).
+              to receive(:new).and_return(double(execute: true))
+
+            expect { post :create, format: :js }.not_to change(Namespace, :count)
+          end
+
+          it "takes the current user's namespace" do
+            expect(Gitlab::GithubImport::ProjectCreator).
+              to receive(:new).with(github_repo, user.namespace, user, access_params).
+              and_return(double(execute: true))
+
+            post :create, format: :js
+          end
         end
       end
     end
diff --git a/spec/controllers/import/gitlab_controller_spec.rb b/spec/controllers/import/gitlab_controller_spec.rb
index e8cf6aa7767..6f75ebb16c8 100644
--- a/spec/controllers/import/gitlab_controller_spec.rb
+++ b/spec/controllers/import/gitlab_controller_spec.rb
@@ -136,21 +136,42 @@ describe Import::GitlabController do
       end
 
       context "when a namespace with the GitLab.com user's username doesn't exist" do
-        it "creates the namespace" do
-          expect(Gitlab::GitlabImport::ProjectCreator).
-            to receive(:new).and_return(double(execute: true))
+        context "when current user can create namespaces" do
+          it "creates the namespace" do
+            expect(Gitlab::GitlabImport::ProjectCreator).
+              to receive(:new).and_return(double(execute: true))
 
-          post :create, format: :js
+            expect { post :create, format: :js }.to change(Namespace, :count).by(1)
+          end
+
+          it "takes the new namespace" do
+            expect(Gitlab::GitlabImport::ProjectCreator).
+              to receive(:new).with(gitlab_repo, an_instance_of(Group), user, access_params).
+              and_return(double(execute: true))
 
-          expect(Namespace.where(name: other_username).first).not_to be_nil
+            post :create, format: :js
+          end
         end
 
-        it "takes the new namespace" do
-          expect(Gitlab::GitlabImport::ProjectCreator).
-            to receive(:new).with(gitlab_repo, an_instance_of(Group), user, access_params).
-            and_return(double(execute: true))
+        context "when current user can't create namespaces" do
+          before do
+            user.update_attribute(:can_create_group, false)
+          end
 
-          post :create, format: :js
+          it "doesn't create the namespace" do
+            expect(Gitlab::GitlabImport::ProjectCreator).
+              to receive(:new).and_return(double(execute: true))
+
+            expect { post :create, format: :js }.not_to change(Namespace, :count)
+          end
+
+          it "takes the current user's namespace" do
+            expect(Gitlab::GitlabImport::ProjectCreator).
+              to receive(:new).with(gitlab_repo, user.namespace, user, access_params).
+              and_return(double(execute: true))
+
+            post :create, format: :js
+          end
         end
       end
     end
diff --git a/spec/helpers/import_helper_spec.rb b/spec/helpers/import_helper_spec.rb
index 3391234e9f5..187b891b927 100644
--- a/spec/helpers/import_helper_spec.rb
+++ b/spec/helpers/import_helper_spec.rb
@@ -1,6 +1,30 @@
 require 'rails_helper'
 
 describe ImportHelper do
+  describe '#import_project_target' do
+    let(:user) { create(:user) }
+
+    before do
+      allow(helper).to receive(:current_user).and_return(user)
+    end
+
+    context 'when current user can create namespaces' do
+      it 'returns project namespace' do
+        user.update_attribute(:can_create_group, true)
+
+        expect(helper.import_project_target('asd', 'vim')).to eq 'asd/vim'
+      end
+    end
+
+    context 'when current user can not create namespaces' do
+      it "takes the current user's namespace" do
+        user.update_attribute(:can_create_group, false)
+
+        expect(helper.import_project_target('asd', 'vim')).to eq "#{user.namespace_path}/vim"
+      end
+    end
+  end
+
   describe '#github_project_link' do
     context 'when provider does not specify a custom URL' do
       it 'uses default GitHub URL' do