Add specs

app/controllers/projects/git_http_controller.rb

@@ -31,6 +31,8 @@ class Projects::GitHttpController < Projects::GitHttpClientController
   # POST /foo/bar.git/git-receive-pack" (git push)
   def git_receive_pack
+    raise Gitlab::GitAccess::NotFoundError, 'Could not create project' unless project
+
     render_ok
   end

changelogs/unreleased/26388-push-to-create-a-new-project.yml

+---
+title: User can now git push to create a new project
+merge_request: 16547
+author:
+type: added

lib/api/helpers/internal_helpers.rb

@@ -64,6 +64,15 @@ module API
         false
       end
+      def project_params
+        {
+          description: "",
+          path: Project.parse_project_id(project_match[:project_id]),
+          namespace_id: project_namespace&.id,
+          visibility_level: Gitlab::VisibilityLevel::PRIVATE.to_s
+        }
+      end
+
       private
       def project_path_regex
@@ -71,11 +80,13 @@ module API
       end
       def project_match
-        @match ||= params[:project].match(project_path_regex).captures
+        @project_match ||= params[:project].match(project_path_regex)
       end
-      def namespace
-        @namespace ||= Namespace.find_by_path_or_name(project_match[:namespace_id])
+      def project_namespace
+        return unless project_match
+
+        @project_namespace ||= Namespace.find_by_path_or_name(project_match[:namespace_id])
       end
       # rubocop:disable Gitlab/ModuleWithInstanceVariables

lib/api/internal.rb

@@ -43,7 +43,7 @@ module API
         access_checker_klass = wiki? ? Gitlab::GitAccessWiki : Gitlab::GitAccess
         access_checker = access_checker_klass
-          .new(actor, project, protocol, authentication_abilities: ssh_authentication_abilities, redirected_path: redirected_path, target_namespace: namespace)
+          .new(actor, project, protocol, authentication_abilities: ssh_authentication_abilities, redirected_path: redirected_path, target_namespace: project_namespace)
         begin
           access_checker.check(params[:action], params[:changes])
@@ -52,13 +52,6 @@ module API
         end
         if user && project.blank? && receive_pack?
-          project_params = {
-            description: "",
-            path: Project.parse_project_id(project_match[:project_name]),
-            namespace_id: namespace&.id,
-            visibility_level: Gitlab::VisibilityLevel::PRIVATE.to_s
-          }
-
           @project = ::Projects::CreateService.new(user, project_params).execute
           if @project.saved?

lib/gitlab/checks/new_project.rb

@@ -20,6 +20,8 @@ module Gitlab
       end
       def add_new_project_message
+        return unless user.present? && project.present?
+
         Gitlab::Redis::SharedState.with do |redis|
           key = self.class.new_project_message_key(user.id, project.id)
           redis.setex(key, 5.minutes, new_project_message)

lib/gitlab/git_access.rb

@@ -19,8 +19,7 @@ module Gitlab
       upload_pack_disabled_over_http: 'Pulling over HTTP is not allowed.',
       receive_pack_disabled_over_http: 'Pushing over HTTP is not allowed.',
       read_only: 'The repository is temporarily read-only. Please try again later.',
-      cannot_push_to_read_only: "You can't push code to a read-only GitLab instance.",
-      create: "Creating a repository to that namespace is not allowed."
+      cannot_push_to_read_only: "You can't push code to a read-only GitLab instance."
     }.freeze
     DOWNLOAD_COMMANDS = %w{ git-upload-pack git-upload-archive }.freeze
@@ -53,7 +52,7 @@ module Gitlab
         check_download_access!
       when *PUSH_COMMANDS
         check_push_access!(cmd, changes)
-        check_repository_creation!(cmd)
+        check_namespace_accessibility!(cmd)
       end
       true
@@ -149,16 +148,12 @@ module Gitlab
       end
     end
-    def check_repository_creation!(cmd)
+    def check_namespace_accessibility!(cmd)
       return unless project.blank?
       unless target_namespace
         raise NotFoundError, ERROR_MESSAGES[:namespace_not_found]
       end
-
-      unless can_create_project_in_namespace?(cmd)
-        raise UnauthorizedError, ERROR_MESSAGES[:create]
-      end
     end
     def check_download_access!

spec/lib/gitlab/checks/new_project_spec.rb

+require 'rails_helper'
+
+describe Gitlab::Checks::NewProject, :clean_gitlab_redis_shared_state do
+  let(:user) { create(:user) }
+  let(:project) { create(:project) }
+
+  describe '.fetch_new_project_message' do
+    context 'with a new project message queue' do
+      let(:new_project) { described_class.new(user, project, 'http') }
+
+      before do
+        new_project.add_new_project_message
+      end
+
+      it 'returns new project message' do
+        expect(described_class.fetch_new_project_message(user.id, project.id)).to eq(new_project.new_project_message)
+      end
+
+      it 'deletes the new project message from redis' do
+        expect(Gitlab::Redis::SharedState.with { |redis| redis.get("new_project:#{user.id}:#{project.id}") }).not_to be_nil
+        described_class.fetch_new_project_message(user.id, project.id)
+        expect(Gitlab::Redis::SharedState.with { |redis| redis.get("new_project:#{user.id}:#{project.id}") }).to be_nil
+      end
+    end
+
+    context 'with no new project message queue' do
+      it 'returns nil' do
+        expect(described_class.fetch_new_project_message(1, 2)).to be_nil
+      end
+    end
+  end
+
+  describe '#add_new_project_message' do
+    it 'queues a new project message' do
+      new_project = described_class.new(user, project, 'http')
+
+      expect(new_project.add_new_project_message).to eq('OK')
+    end
+
+    it 'handles anonymous push' do
+      new_project = described_class.new(user, nil, 'http')
+
+      expect(new_project.add_new_project_message).to be_nil
+    end
+  end
+end

spec/lib/gitlab/checks/project_moved_spec.rb

@@ -6,14 +6,14 @@ describe Gitlab::Checks::ProjectMoved, :clean_gitlab_redis_shared_state do
   describe '.fetch_redirct_message' do
     context 'with a redirect message queue' do
-      it 'should return the redirect message' do
+      it 'returns the redirect message' do
         project_moved = described_class.new(project, user, 'foo/bar', 'http')
         project_moved.add_redirect_message
         expect(described_class.fetch_redirect_message(user.id, project.id)).to eq(project_moved.redirect_message)
       end
-      it 'should delete the redirect message from redis' do
+      it 'deletes the redirect message from redis' do
         project_moved = described_class.new(project, user, 'foo/bar', 'http')
         project_moved.add_redirect_message
@@ -24,19 +24,19 @@ describe Gitlab::Checks::ProjectMoved, :clean_gitlab_redis_shared_state do
     end
     context 'with no redirect message queue' do
-      it 'should return nil' do
+      it 'returns nil' do
         expect(described_class.fetch_redirect_message(1, 2)).to be_nil
       end
     end
   end
   describe '#add_redirect_message' do
-    it 'should queue a redirect message' do
+    it 'queues a redirect message' do
       project_moved = described_class.new(project, user, 'foo/bar', 'http')
       expect(project_moved.add_redirect_message).to eq("OK")
     end
-    it 'should handle anonymous clones' do
+    it 'handles anonymous clones' do
       project_moved = described_class.new(project, nil, 'foo/bar', 'http')
       expect(project_moved.add_redirect_message).to eq(nil)
@@ -45,7 +45,7 @@ describe Gitlab::Checks::ProjectMoved, :clean_gitlab_redis_shared_state do
   describe '#redirect_message' do
     context 'when the push is rejected' do
-      it 'should return a redirect message telling the user to try again' do
+      it 'returns a redirect message telling the user to try again' do
         project_moved = described_class.new(project, user, 'foo/bar', 'http')
         message = "Project 'foo/bar' was moved to '#{project.full_path}'." +
           "\n\nPlease update your Git remote:" +
@@ -56,7 +56,7 @@ describe Gitlab::Checks::ProjectMoved, :clean_gitlab_redis_shared_state do
     end
     context 'when the push is not rejected' do
-      it 'should return a redirect message' do
+      it 'returns a redirect message' do
         project_moved = described_class.new(project, user, 'foo/bar', 'http')
         message = "Project 'foo/bar' was moved to '#{project.full_path}'." +
           "\n\nPlease update your Git remote:" +
@@ -69,7 +69,7 @@ describe Gitlab::Checks::ProjectMoved, :clean_gitlab_redis_shared_state do
   describe '#permanent_redirect?' do
     context 'with a permanent RedirectRoute' do
-      it 'should return true' do
+      it 'returns true' do
         project.route.create_redirect('foo/bar', permanent: true)
         project_moved = described_class.new(project, user, 'foo/bar', 'http')
         expect(project_moved.permanent_redirect?).to be_truthy
@@ -77,7 +77,7 @@ describe Gitlab::Checks::ProjectMoved, :clean_gitlab_redis_shared_state do
     end
     context 'without a permanent RedirectRoute' do
-      it 'should return false' do
+      it 'returns false' do
         project.route.create_redirect('foo/bar')
         project_moved = described_class.new(project, user, 'foo/bar', 'http')
         expect(project_moved.permanent_redirect?).to be_falsy

spec/lib/gitlab/git_access_spec.rb

@@ -152,6 +152,30 @@ describe Gitlab::GitAccess do
           expect { push_access_check }.to raise_not_found
         end
       end
+
+      context 'when user is allowed to create project in namespace' do
+        let(:access) { described_class.new(actor, nil, protocol, authentication_abilities: authentication_abilities, redirected_path: redirected_path, target_namespace: user.namespace) }
+
+        it 'blocks pull access with "not found"' do
+          expect { pull_access_check }.to raise_not_found
+        end
+
+        it 'allows push access' do
+          expect { push_access_check }.not_to raise_error
+        end
+      end
+
+      context 'when user is not allowed to create project in namespace' do
+        let(:user2) { create(:user) }
+        let(:access) { described_class.new(actor, nil, protocol, authentication_abilities: authentication_abilities, redirected_path: redirected_path, target_namespace: user2.namespace) }
+
+        it 'blocks push and pull with "not found"' do
+          aggregate_failures do
+            expect { pull_access_check }.to raise_not_found
+            expect { push_access_check }.to raise_not_found
+          end
+        end
+      end
     end
   end
@@ -311,6 +335,51 @@ describe Gitlab::GitAccess do
     end
   end
+  describe '#check_namespace_accessibility!' do
+    context 'when project exists' do
+      context 'when user can pull or push' do
+        before do
+          project.add_master(user)
+        end
+
+        it 'does not block pull or push' do
+          aggregate_failures do
+            expect { push_access_check }.not_to raise_error
+            expect { pull_access_check }.not_to raise_error
+          end
+        end
+      end
+    end
+
+    context 'when project does not exist' do
+      context 'when namespace does not exist' do
+        let(:access) { described_class.new(actor, nil, protocol, authentication_abilities: authentication_abilities, redirected_path: redirected_path, target_namespace: nil) }
+
+        it 'blocks push and pull' do
+          aggregate_failures do
+            expect { push_access_check }.not_to raise_namespace_not_found
+            expect { pull_access_check }.not_to raise_namespace_not_found
+          end
+        end
+      end
+
+      context 'when namespace exists' do
+        context 'when user is unable to push to namespace' do
+          let(:user2) { create(:user) }
+          let(:access) { described_class.new(actor, nil, protocol, authentication_abilities: authentication_abilities, redirected_path: redirected_path, target_namespace: user2.namespace) }
+
+          it 'blocks push' do
+            expect { push_access_check }.to raise_project_create
+          end
+
+          it 'does not block pull' do
+            expect { push_access_check }.to raise_error
+          end
+        end
+      end
+    end
+  end
+
   describe '#check_download_access!' do
     it 'allows masters to pull' do
       project.add_master(user)
@@ -773,6 +842,16 @@ describe Gitlab::GitAccess do
                 Gitlab::GitAccess::ERROR_MESSAGES[:project_not_found])
   end
+  def raise_namespace_not_found
+    raise_error(Gitlab::GitAccess::NotFoundError,
+                Gitlab::GitAccess::ERROR_MESSAGES[:namespace_not_found])
+  end
+
+  def raise_project_create
+    raise_error(Gitlab::GitAccess::NotFoundError,
+                Gitlab::GitAccess::ERROR_MESSAGES[:create])
+  end
+
   def build_authentication_abilities
     [
       :read_project,

spec/models/project_spec.rb

@@ -1079,6 +1079,12 @@ describe Project do
     end
   end
+  describe '.parse_project_id' do
+    it 'removes .git from the project_id' do
+      expect(described_class.parse_project_id('new-project.git')).to eq('new-project')
+    end
+  end
+
   context 'repository storage by default' do
     let(:project) { create(:project) }

spec/requests/api/internal_spec.rb

@@ -366,25 +366,28 @@ describe API::Internal do
           end
         end
-        context 'project as /namespace/project' do
+        context 'project as namespace/project' do
           it do
-            pull(key, project_with_repo_path('/' + project.full_path))
+            push(key, project_with_repo_path(project.full_path))
             expect(response).to have_gitlab_http_status(200)
             expect(json_response["status"]).to be_truthy
             expect(json_response["repository_path"]).to eq(project.repository.path_to_repo)
             expect(json_response["gl_repository"]).to eq("project-#{project.id}")
           end
-        end
-        context 'project as namespace/project' do
-          it do
-            pull(key, project_with_repo_path(project.full_path))
-            expect(response).to have_gitlab_http_status(200)
-            expect(json_response["status"]).to be_truthy
-            expect(json_response["repository_path"]).to eq(project.repository.path_to_repo)
-            expect(json_response["gl_repository"]).to eq("project-#{project.id}")
+          context 'when project does not exist' do
+            it 'creates a new project' do
+              path = "#{user.namespace.path}/notexist.git"
+              expect do
+                push_with_path(key, path)
+              end.to change { Project.count }.by(1)
+
+              expect(response).to have_gitlab_http_status(200)
+              expect(json_response["status"]).to be_truthy
+              expect(json_response["gitaly"]["repository"]["relative_path"]).to eq(path)
+            end
           end
         end
       end
@@ -818,6 +821,19 @@ describe API::Internal do
       end
     end
+    context 'with new project data' do
+      it 'returns new project message on the response' do
+        new_project = Gitlab::Checks::NewProject.new(user, project, 'http')
+        new_project.add_new_project_message
+
+        post api("/internal/post_receive"), valid_params
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response["new_project_message"]).to be_present
+        expect(json_response["new_project_message"]).to eq(new_project.new_project_message)
+      end
+    end
+
     context 'with an orphaned write deploy key' do
       it 'does not try to notify that project moved' do
         allow_any_instance_of(Gitlab::Identifier).to receive(:identify).and_return(nil)

spec/requests/git_http_spec.rb

@@ -107,15 +107,39 @@ describe 'Git HTTP requests' do
     let(:user) { create(:user) }
     context "when the project doesn't exist" do
-      let(:path) { 'doesnt/exist.git' }
-      it_behaves_like 'pulls require Basic HTTP Authentication'
-      it_behaves_like 'pushes require Basic HTTP Authentication'
-      context 'when authenticated' do
-        it 'rejects downloads and uploads with 404 Not Found' do
-          download_or_upload(path, user: user.username, password: user.password) do |response|
-            expect(response).to have_gitlab_http_status(:not_found)
+      context "when namespace doesn't exist" do
+        let(:path) { 'doesnt/exist.git' }
+        it_behaves_like 'pulls require Basic HTTP Authentication'
+        it_behaves_like 'pushes require Basic HTTP Authentication'
+        context 'when authenticated' do
+          it 'rejects downloads and uploads with 404 Not Found' do
+            download_or_upload(path, user: user.username, password: user.password) do |response|
+              expect(response).to have_gitlab_http_status(:not_found)
+            end
+          end
+        end
+      end
+
+      context 'when namespace exists' do
+        let(:path) { "#{user.namespace.path}/new-project.git"}
+
+        context 'when authenticated' do
+          it 'creates a new project under the existing namespace' do
+            expect do
+              upload(path, user: user.username, password: user.password) do |response|
+                expect(response).to have_gitlab_http_status(:ok)
+              end
+            end.to change { user.projects.count }.by(1)
+          end
+
+          it 'rejects upload with 404 Not Found when project is invalid' do
+            path = "#{user.namespace.path}/new.git"
+
+            upload(path, user: user.username, password: user.password) do |response|
+              expect(response).to have_gitlab_http_status(:not_found)
+            end
           end
         end
       end