From 1e56b3f476f9779ec747534e94156a6b8076209c Mon Sep 17 00:00:00 2001 From: Tiago Botelho Date: Fri, 2 Feb 2018 15:27:30 +0000 Subject: [PATCH] Moves project creationg to git access check for git push --- GITLAB_SHELL_VERSION | 2 +- .../projects/git_http_controller.rb | 19 +- .../projects/create_from_push_service.rb | 37 --- doc/gitlab-basics/create-project.md | 17 +- lib/api/helpers/internal_helpers.rb | 20 +- lib/api/internal.rb | 15 +- .../{base_project.rb => post_push_message.rb} | 2 +- lib/gitlab/checks/project_created.rb | 6 +- lib/gitlab/checks/project_moved.rb | 4 +- lib/gitlab/git_access.rb | 92 ++++--- lib/gitlab/path_regex.rb | 2 +- lib/gitlab/user_access.rb | 3 +- spec/lib/gitlab/git_access_spec.rb | 226 ++++++++++++++---- spec/requests/api/internal_spec.rb | 26 -- .../projects/create_from_push_service_spec.rb | 34 --- 15 files changed, 272 insertions(+), 233 deletions(-) delete mode 100644 app/services/projects/create_from_push_service.rb rename lib/gitlab/checks/{base_project.rb => post_push_message.rb} (97%) delete mode 100644 spec/services/projects/create_from_push_service_spec.rb diff --git a/GITLAB_SHELL_VERSION b/GITLAB_SHELL_VERSION index 9b9a244206f..090ea9dad19 100644 --- a/GITLAB_SHELL_VERSION +++ b/GITLAB_SHELL_VERSION @@ -1 +1 @@ -6.0.2 +6.0.3 diff --git a/app/controllers/projects/git_http_controller.rb b/app/controllers/projects/git_http_controller.rb index 90a9079fab3..45910a9be44 100644 --- a/app/controllers/projects/git_http_controller.rb +++ b/app/controllers/projects/git_http_controller.rb @@ -11,7 +11,6 @@ class Projects::GitHttpController < Projects::GitHttpClientController # GET /foo/bar.git/info/refs?service=git-receive-pack (git push) def info_refs log_user_activity if upload_pack? - create_new_project if receive_pack? && project.blank? render_ok end @@ -36,10 +35,6 @@ class Projects::GitHttpController < Projects::GitHttpClientController git_command == 'git-upload-pack' end - def receive_pack? - git_command == 'git-receive-pack' - end - def git_command if action_name == 'info_refs' params[:service] @@ -48,10 +43,6 @@ class Projects::GitHttpController < Projects::GitHttpClientController end end - def create_new_project - @project = ::Projects::CreateFromPushService.new(user, params[:project_id], namespace, 'http').execute - end - def render_ok set_workhorse_internal_api_content_type render json: Gitlab::Workhorse.git_http_ok(repository, wiki?, user, action_name) @@ -70,7 +61,10 @@ class Projects::GitHttpController < Projects::GitHttpClientController end def access - @access ||= access_klass.new(access_actor, project, 'http', authentication_abilities: authentication_abilities, redirected_path: redirected_path, target_namespace: namespace) + @access ||= access_klass.new(access_actor, project, + 'http', authentication_abilities: authentication_abilities, + namespace_path: params[:namespace_id], project_path: project_path, + redirected_path: redirected_path) end def access_actor @@ -82,14 +76,15 @@ class Projects::GitHttpController < Projects::GitHttpClientController # Use the magic string '_any' to indicate we do not know what the # changes are. This is also what gitlab-shell does. access.check(git_command, '_any') + @project ||= access.project end def access_klass @access_klass ||= wiki? ? Gitlab::GitAccessWiki : Gitlab::GitAccess end - def namespace - @namespace ||= Namespace.find_by_full_path(params[:namespace_id]) + def project_path + @project_path ||= params[:project_id].sub(/\.git$/, '') end def log_user_activity diff --git a/app/services/projects/create_from_push_service.rb b/app/services/projects/create_from_push_service.rb deleted file mode 100644 index 949a0bfdc09..00000000000 --- a/app/services/projects/create_from_push_service.rb +++ /dev/null @@ -1,37 +0,0 @@ -module Projects - class CreateFromPushService < BaseService - attr_reader :user, :project_path, :namespace, :protocol - - def initialize(user, project_path, namespace, protocol) - @user = user - @project_path = project_path - @namespace = namespace - @protocol = protocol - end - - def execute - return unless user - - project = Projects::CreateService.new(user, project_params).execute - - if project.saved? - Gitlab::Checks::ProjectCreated.new(project, user, protocol).add_message - else - raise Gitlab::GitAccess::ProjectCreationError, "Could not create project: #{project.errors.full_messages.join(', ')}" - end - - project - end - - private - - def project_params - { - description: "", - path: project_path.gsub(/\.git$/, ''), - namespace_id: namespace&.id, - visibility_level: Gitlab::VisibilityLevel::PRIVATE.to_s - } - end - end -end diff --git a/doc/gitlab-basics/create-project.md b/doc/gitlab-basics/create-project.md index d491d676884..7b87039da84 100644 --- a/doc/gitlab-basics/create-project.md +++ b/doc/gitlab-basics/create-project.md @@ -39,18 +39,18 @@ When you create a new repo locally, instead of going to GitLab to manually create a new project and then push the repo, you can directly push it to -GitLab to create the new project, all without leaving your terminal. That -will automatically create a new project under a GitLab namespace that you have access to -with its visibility set to private by default (you can later change it). +GitLab to create the new project, all without leaving your terminal. If you have access to that +namespace, we will automatically create a new project under that GitLab namespace with its +visibility set to private by default (you can later change it in the UI). This can be done by using either SSH or HTTP: ``` ## Git push using SSH -git push git@gitlab.com:namespace/nonexistent-project.git branch_name +git push git@gitlab.example.com:namespace/nonexistent-project.git ## Git push using HTTP -git push https://gitlab.com/namespace/nonexistent-project.git branch_name +git push https://gitlab.example.com/namespace/nonexistent-project.git ``` Once the push finishes successfully, a remote message will indicate @@ -61,15 +61,12 @@ remote: remote: The private project namespace/nonexistent-project was created. remote: remote: To configure the remote, run: -remote: git remote add origin https://gitlab.com/namespace/nonexistent-project.git +remote: git remote add origin https://gitlab.example.com/namespace/nonexistent-project.git remote: remote: To view the project, visit: -remote: https://gitlab.com/namespace/nonexistent-project +remote: https://gitlab.example.com/namespace/nonexistent-project remote: ``` -If the project name is already in use, your push will be rejected -to prevent accidental overwriting the existing project. - [import it]: ../workflow/importing/README.md [reserved]: ../user/reserved_names.md diff --git a/lib/api/helpers/internal_helpers.rb b/lib/api/helpers/internal_helpers.rb index bff245fe9a2..cd59da6fc70 100644 --- a/lib/api/helpers/internal_helpers.rb +++ b/lib/api/helpers/internal_helpers.rb @@ -1,8 +1,6 @@ module API module Helpers module InternalHelpers - include Gitlab::Utils::StrongMemoize - attr_reader :redirected_path def wiki? @@ -49,10 +47,6 @@ module API ::Users::ActivityService.new(actor, 'Git SSH').execute if commands.include?(params[:action]) end - def receive_pack? - params[:action] == 'git-receive-pack' - end - def merge_request_urls ::MergeRequests::GetUrlsService.new(project).execute(params[:changes]) end @@ -66,16 +60,18 @@ module API false end - def project_namespace - strong_memoize(:project_namespace) do - project&.namespace || Namespace.find_by_full_path(project_match[:namespace_path]) - end + def project_path + project&.path || project_path_match[:project_path] + end + + def namespace_path + project&.namespace&.full_path || project_path_match[:namespace_path] end private - def project_match - @project_match ||= params[:project].match(Gitlab::PathRegex.full_project_git_path_regex) || {} + def project_path_match + @project_path_match ||= params[:project].match(Gitlab::PathRegex.full_project_git_path_regex) || {} end # rubocop:disable Gitlab/ModuleWithInstanceVariables diff --git a/lib/api/internal.rb b/lib/api/internal.rb index ed6d022df97..9285fb90cdc 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -42,23 +42,18 @@ module API end 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: project_namespace) + access_checker = access_checker_klass.new(actor, project, + protocol, authentication_abilities: ssh_authentication_abilities, + namespace_path: namespace_path, project_path: project_path, + redirected_path: redirected_path) begin access_checker.check(params[:action], params[:changes]) + @project ||= access_checker.project rescue Gitlab::GitAccess::UnauthorizedError, Gitlab::GitAccess::NotFoundError => e return { status: false, message: e.message } end - if receive_pack? && project.blank? - begin - @project = ::Projects::CreateFromPushService.new(user, project_match[:project_path], project_namespace, protocol).execute - rescue Gitlab::GitAccess::ProjectCreationError => e - return { status: false, message: e.message } - end - end - log_user_activity(actor) { diff --git a/lib/gitlab/checks/base_project.rb b/lib/gitlab/checks/post_push_message.rb similarity index 97% rename from lib/gitlab/checks/base_project.rb rename to lib/gitlab/checks/post_push_message.rb index dd6c007b356..473c0385b34 100644 --- a/lib/gitlab/checks/base_project.rb +++ b/lib/gitlab/checks/post_push_message.rb @@ -1,6 +1,6 @@ module Gitlab module Checks - class BaseProject + class PostPushMessage def initialize(project, user, protocol) @project = project @user = user diff --git a/lib/gitlab/checks/project_created.rb b/lib/gitlab/checks/project_created.rb index bd1e204bc81..cec270d6a58 100644 --- a/lib/gitlab/checks/project_created.rb +++ b/lib/gitlab/checks/project_created.rb @@ -1,12 +1,12 @@ module Gitlab module Checks - class ProjectCreated < BaseProject + class ProjectCreated < PostPushMessage PROJECT_CREATED = "project_created".freeze def message - <<~MESSAGE.strip_heredoc + <<~MESSAGE - The private project #{project.full_path} was created. + The private project #{project.full_path} was successfully created. To configure the remote, run: git remote add origin #{url_to_repo} diff --git a/lib/gitlab/checks/project_moved.rb b/lib/gitlab/checks/project_moved.rb index eca59e88e24..3263790a876 100644 --- a/lib/gitlab/checks/project_moved.rb +++ b/lib/gitlab/checks/project_moved.rb @@ -1,6 +1,6 @@ module Gitlab module Checks - class ProjectMoved < BaseProject + class ProjectMoved < PostPushMessage REDIRECT_NAMESPACE = "redirect_namespace".freeze def initialize(project, user, protocol, redirected_path) @@ -10,7 +10,7 @@ module Gitlab end def message(rejected: false) - <<~MESSAGE.strip_heredoc + <<~MESSAGE Project '#{redirected_path}' was moved to '#{project.full_path}'. Please update your Git remote: diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index 84299dd5790..bc1e83f77b2 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -28,32 +28,37 @@ module Gitlab PUSH_COMMANDS = %w{ git-receive-pack }.freeze ALL_COMMANDS = DOWNLOAD_COMMANDS + PUSH_COMMANDS - attr_reader :actor, :project, :protocol, :authentication_abilities, :redirected_path, :target_namespace + attr_reader :actor, :project, :protocol, :authentication_abilities, :namespace_path, :project_path, :redirected_path - def initialize(actor, project, protocol, authentication_abilities:, redirected_path: nil, target_namespace: nil) + def initialize(actor, project, protocol, authentication_abilities:, namespace_path: nil, project_path: nil, redirected_path: nil) @actor = actor @project = project @protocol = protocol - @redirected_path = redirected_path @authentication_abilities = authentication_abilities - @target_namespace = target_namespace + @namespace_path = namespace_path + @project_path = project_path + @redirected_path = redirected_path end def check(cmd, changes) check_protocol! check_valid_actor! check_active_user! - check_project_accessibility!(cmd) - check_project_moved! check_command_disabled!(cmd) check_command_existence!(cmd) - check_repository_existence!(cmd) + check_db_accessibility!(cmd) + + ensure_project_on_push!(cmd, changes) + + check_project_accessibility! + check_project_moved! + check_repository_existence! case cmd when *DOWNLOAD_COMMANDS check_download_access! when *PUSH_COMMANDS - check_push_access!(cmd, changes) + check_push_access!(changes) end true @@ -99,8 +104,8 @@ module Gitlab end end - def check_project_accessibility!(cmd) - unless can_create_project_in_namespace?(cmd) || can_read_project? + def check_project_accessibility! + if project.blank? || !can_read_project? raise NotFoundError, ERROR_MESSAGES[:project_not_found] end end @@ -143,16 +148,49 @@ module Gitlab end end - def check_repository_existence!(cmd) - unless can_create_project_in_namespace?(cmd) || project.repository.exists? + def check_db_accessibility!(cmd) + return unless receive_pack?(cmd) + + if Gitlab::Database.read_only? + raise UnauthorizedError, push_to_read_only_message + end + end + + def ensure_project_on_push!(cmd, changes) + return if project || deploy_key? + return unless receive_pack?(cmd) && changes == '_any' && authentication_abilities.include?(:push_code) + + namespace = Namespace.find_by_full_path(namespace_path) + + return unless user&.can?(:create_projects, namespace) + + project_params = { + path: project_path, + namespace_id: namespace.id, + visibility_level: Gitlab::VisibilityLevel::PRIVATE + } + + project = Projects::CreateService.new(user, project_params).execute + + unless project.saved? + raise ProjectCreationError, "Could not create project: #{project.errors.full_messages.join(', ')}" + end + + @project = project + user_access.project = @project + + Checks::ProjectCreated.new(project, user, protocol).add_message + end + + def check_repository_existence! + unless project.repository.exists? raise UnauthorizedError, ERROR_MESSAGES[:no_repo] end end def check_download_access! - return if deploy_key? - - passed = user_can_download_code? || + passed = deploy_key? || + user_can_download_code? || build_can_download_code? || guest_can_download_code? @@ -161,13 +199,7 @@ module Gitlab end end - def check_push_access!(cmd, changes) - if Gitlab::Database.read_only? - raise UnauthorizedError, push_to_read_only_message - end - - return if can_create_project_in_namespace?(cmd) - + def check_push_access!(changes) if project.repository_read_only? raise UnauthorizedError, ERROR_MESSAGES[:read_only] end @@ -180,8 +212,6 @@ module Gitlab raise UnauthorizedError, ERROR_MESSAGES[:upload] end - return if changes.blank? # Allow access. - check_change_access!(changes) end @@ -198,6 +228,8 @@ module Gitlab end def check_change_access!(changes) + return if changes.blank? # Allow access. + changes_list = Gitlab::ChangesList.new(changes) # Iterate over all changes to find if user allowed all of them to be applied @@ -240,14 +272,6 @@ module Gitlab end || Guest.can?(:read_project, project) end - def can_create_project_in_namespace?(cmd) - strong_memoize(:can_create_project_in_namespace) do - return false unless push?(cmd) && target_namespace && project.blank? - - user.can?(:create_projects, target_namespace) - end - end - def http? protocol == 'http' end @@ -260,10 +284,6 @@ module Gitlab command == 'git-receive-pack' end - def push?(cmd) - PUSH_COMMANDS.include?(cmd) - end - def upload_pack_disabled_over_http? !Gitlab.config.gitlab_shell.upload_pack end diff --git a/lib/gitlab/path_regex.rb b/lib/gitlab/path_regex.rb index c6a594d38d1..1fc0363904a 100644 --- a/lib/gitlab/path_regex.rb +++ b/lib/gitlab/path_regex.rb @@ -188,7 +188,7 @@ module Gitlab end def full_project_git_path_regex - @full_project_git_path_regex ||= /\A\/?(?#{full_namespace_route_regex})\/(?#{project_git_route_regex})\z/.freeze + @full_project_git_path_regex ||= %r{\A\/?(?#{full_namespace_route_regex})\/(?#{project_route_regex})\.git\z} end def full_namespace_format_regex diff --git a/lib/gitlab/user_access.rb b/lib/gitlab/user_access.rb index f357488ac61..15eb1c41213 100644 --- a/lib/gitlab/user_access.rb +++ b/lib/gitlab/user_access.rb @@ -6,7 +6,8 @@ module Gitlab [user&.id, project&.id] end - attr_reader :user, :project + attr_reader :user + attr_accessor :project def initialize(user, project: nil) @user = user diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index 6bfac67ddae..cc48373a0c0 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -5,11 +5,19 @@ describe Gitlab::GitAccess do let(:actor) { user } let(:project) { create(:project, :repository) } + let(:project_path) { project.path } + let(:namespace_path) { project&.namespace&.path } let(:protocol) { 'ssh' } let(:authentication_abilities) { %i[read_project download_code push_code] } let(:redirected_path) { nil } - let(:access) { described_class.new(actor, project, protocol, authentication_abilities: authentication_abilities, redirected_path: redirected_path) } + let(:access) do + described_class.new(actor, project, + protocol, authentication_abilities: authentication_abilities, + namespace_path: namespace_path, project_path: project_path, + redirected_path: redirected_path) + end + let(:push_access_check) { access.check('git-receive-pack', '_any') } let(:pull_access_check) { access.check('git-upload-pack', '_any') } @@ -145,6 +153,7 @@ describe Gitlab::GitAccess do context 'when the project is nil' do let(:project) { nil } + let(:project_path) { "new-project" } it 'blocks push and pull with "not found"' do aggregate_failures do @@ -154,7 +163,13 @@ describe Gitlab::GitAccess do 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) } + let(:namespace_path) { user.namespace.path } + let(:access) do + described_class.new(actor, nil, + protocol, authentication_abilities: authentication_abilities, + project_path: project_path, namespace_path: namespace_path, + redirected_path: redirected_path) + end it 'blocks pull access with "not found"' do expect { pull_access_check }.to raise_not_found @@ -167,7 +182,13 @@ describe Gitlab::GitAccess do 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) } + let(:namespace_path) { user2.namespace.path } + let(:access) do + described_class.new(actor, nil, + protocol, authentication_abilities: authentication_abilities, + project_path: project_path, namespace_path: namespace_path, + redirected_path: redirected_path) + end it 'blocks push and pull with "not found"' do aggregate_failures do @@ -297,6 +318,52 @@ describe Gitlab::GitAccess do end end + describe '#check_authentication_abilities!' do + before do + project.add_master(user) + end + + context 'when download' do + let(:authentication_abilities) { [] } + + it 'raises unauthorized with download error' do + expect { pull_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:download]) + end + + context 'when authentication abilities include download code' do + let(:authentication_abilities) { [:download_code] } + + it 'does not raise any errors' do + expect { pull_access_check }.not_to raise_error + end + end + + context 'when authentication abilities include build download code' do + let(:authentication_abilities) { [:build_download_code] } + + it 'does not raise any errors' do + expect { pull_access_check }.not_to raise_error + end + end + end + + context 'when upload' do + let(:authentication_abilities) { [] } + + it 'raises unauthorized with push error' do + expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:upload]) + end + + context 'when authentication abilities include push code' do + let(:authentication_abilities) { [:push_code] } + + it 'does not raise any errors' do + expect { push_access_check }.not_to raise_error + end + end + end + end + describe '#check_command_disabled!' do before do project.add_master(user) @@ -335,34 +402,112 @@ 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) + describe '#check_db_accessibility!' do + context 'when in a read-only GitLab instance' do + before do + create(:protected_branch, name: 'feature', project: project) + allow(Gitlab::Database).to receive(:read_only?) { true } + end + + it { expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:cannot_push_to_read_only]) } + end + end + + describe '#ensure_project_on_push!' do + let(:access) do + described_class.new(actor, project, + protocol, authentication_abilities: authentication_abilities, + project_path: project_path, namespace_path: namespace_path, + redirected_path: redirected_path) + end + + context 'when push' do + let(:cmd) { 'git-receive-pack' } + + context 'when project does not exist' do + let(:project_path) { "nonexistent" } + let(:project) { nil } + + context 'when changes is _any' do + let(:changes) { '_any' } + + context 'when authentication abilities include push code' do + let(:authentication_abilities) { [:push_code] } + + context 'when user can create project in namespace' do + let(:namespace_path) { user.namespace.path } + + it 'creates a new project' do + expect { access.send(:ensure_project_on_push!, cmd, changes) }.to change { Project.count }.by(1) + end + end + + context 'when user cannot create project in namespace' do + let(:user2) { create(:user) } + let(:namespace_path) { user2.namespace.path } + + it 'does not create a new project' do + expect { access.send(:ensure_project_on_push!, cmd, changes) }.not_to change { Project.count } + end + end + end + + context 'when authentication abilities do not include push code' do + let(:authentication_abilities) { [] } + + context 'when user can create project in namespace' do + let(:namespace_path) { user.namespace.path } + + it 'does not create a new project' do + expect { access.send(:ensure_project_on_push!, cmd, changes) }.not_to change { Project.count } + end + end + end 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 + context 'when check contains actual changes' do + let(:changes) { "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/new_branch" } + + it 'does not create a new project' do + expect { access.send(:ensure_project_on_push!, cmd, changes) }.not_to change { Project.count } end end end + + context 'when project exists' do + let(:changes) { '_any' } + let!(:project) { create(:project) } + + it 'does not create a new project' do + expect { access.send(:ensure_project_on_push!, cmd, changes) }.not_to change { Project.count } + end + end + + context 'when deploy key is used' do + let(:key) { create(:deploy_key, user: user) } + let(:actor) { key } + let(:project_path) { "nonexistent" } + let(:project) { nil } + let(:namespace_path) { user.namespace.path } + let(:changes) { '_any' } + + it 'does not create a new project' do + expect { access.send(:ensure_project_on_push!, cmd, changes) }.not_to change { Project.count } + end + end end - context 'when project does not exist' do - 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) } + context 'when pull' do + let(:cmd) { 'git-upload-pack' } + let(:changes) { '_any' } - it 'blocks push and pull' do - aggregate_failures do - expect { push_access_check }.to raise_not_found - expect { pull_access_check }.to raise_not_found - end - end + context 'when project does not exist' do + let(:project_path) { "new-project" } + let(:namespace_path) { user.namespace.path } + let(:project) { nil } + + it 'does not create a new project' do + expect { access.send(:ensure_project_on_push!, cmd, changes) }.not_to change { Project.count } end end end @@ -395,7 +540,9 @@ describe Gitlab::GitAccess do context 'when project is public' do let(:public_project) { create(:project, :public, :repository) } - let(:access) { described_class.new(nil, public_project, 'web', authentication_abilities: []) } + let(:project_path) { public_project.path } + let(:namespace_path) { public_project.namespace.path } + let(:access) { described_class.new(nil, public_project, 'web', authentication_abilities: [:download_code], project_path: project_path, namespace_path: namespace_path) } context 'when repository is enabled' do it 'give access to download code' do @@ -558,17 +705,15 @@ describe Gitlab::GitAccess do end aggregate_failures do - Gitlab::GitAccess::ALL_COMMANDS.each do |cmd| - matrix.each do |action, allowed| - check = -> { access.send(:check_push_access!, cmd, changes[action]) } + matrix.each do |action, allowed| + check = -> { access.send(:check_push_access!, changes[action]) } - if allowed - expect(&check).not_to raise_error, - -> { "expected #{action} to be allowed" } - else - expect(&check).to raise_error(Gitlab::GitAccess::UnauthorizedError), - -> { "expected #{action} to be disallowed" } - end + if allowed + expect(&check).not_to raise_error, + -> { "expected #{action} to be allowed" } + else + expect(&check).to raise_error(Gitlab::GitAccess::UnauthorizedError), + -> { "expected #{action} to be disallowed" } end end end @@ -697,19 +842,6 @@ describe Gitlab::GitAccess do admin: { push_protected_branch: false, push_all: false, merge_into_protected_branch: false })) end end - - context "when in a read-only GitLab instance" do - before do - create(:protected_branch, name: 'feature', project: project) - allow(Gitlab::Database).to receive(:read_only?) { true } - end - - # Only check admin; if an admin can't do it, other roles can't either - matrix = permissions_matrix[:admin].dup - matrix.each { |key, _| matrix[key] = false } - - run_permission_checks(admin: matrix) - end end describe 'build authentication abilities' do diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb index 5ef180348aa..ea6b0a71849 100644 --- a/spec/requests/api/internal_spec.rb +++ b/spec/requests/api/internal_spec.rb @@ -386,32 +386,6 @@ describe API::Internal do expect(json_response["repository_path"]).to eq(project.repository.path_to_repo) expect(json_response["gl_repository"]).to eq("project-#{project.id}") end - - 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 - - it 'handles project creation failure' do - path = "#{user.namespace.path}/new.git" - - expect do - push_with_path(key, path) - end.not_to change { Project.count } - - expect(response).to have_gitlab_http_status(200) - expect(json_response["status"]).to be_falsey - expect(json_response["message"]).to eq("Could not create project: Path new is a reserved name") - end - end end end end diff --git a/spec/services/projects/create_from_push_service_spec.rb b/spec/services/projects/create_from_push_service_spec.rb deleted file mode 100644 index 329279ea5fe..00000000000 --- a/spec/services/projects/create_from_push_service_spec.rb +++ /dev/null @@ -1,34 +0,0 @@ -require 'spec_helper' - -describe Projects::CreateFromPushService do - let(:user) { create(:user) } - let(:project_path) { "nonexist" } - let(:namespace) { user&.namespace } - let(:protocol) { 'http' } - - subject { described_class.new(user, project_path, namespace, protocol) } - - it 'creates project' do - expect_any_instance_of(Projects::CreateService).to receive(:execute).and_call_original - - expect { subject.execute }.to change { Project.count }.by(1) - end - - it 'raises project creation error when project creation fails' do - allow_any_instance_of(Project).to receive(:saved?).and_return(false) - - expect { subject.execute }.to raise_error(Gitlab::GitAccess::ProjectCreationError) - end - - context 'when user is nil' do - let(:user) { nil } - - subject { described_class.new(user, project_path, namespace, protocol) } - - it 'returns nil' do - expect_any_instance_of(Projects::CreateService).not_to receive(:execute) - - expect(subject.execute).to be_nil - end - end -end