From 5fe06d7365f5552904add8027309d6216954793e Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Thu, 24 Mar 2016 18:58:29 +0100 Subject: [PATCH] Add some upload specs --- .../projects/git_http_controller.rb | 40 +++++++++--- spec/requests/git_http_spec.rb | 63 ++++++++++++++++++- 2 files changed, 91 insertions(+), 12 deletions(-) diff --git a/app/controllers/projects/git_http_controller.rb b/app/controllers/projects/git_http_controller.rb index 129e87dbf13..a26ab736115 100644 --- a/app/controllers/projects/git_http_controller.rb +++ b/app/controllers/projects/git_http_controller.rb @@ -5,10 +5,12 @@ class Projects::GitHttpController < Projects::ApplicationController def git_rpc if upload_pack? && upload_pack_allowed? - render_ok and return + render_ok + elsif receive_pack? && receive_pack_allowed? + render_ok + else + render_not_found end - - render_not_found end %i{info_refs git_receive_pack git_upload_pack}.each do |method| @@ -30,7 +32,7 @@ class Projects::GitHttpController < Projects::ApplicationController end def project_found? - render_not_found if project.nil? + render_not_found if project.blank? end def ci_request?(login, password) @@ -124,13 +126,21 @@ class Projects::GitHttpController < Projects::ApplicationController end def upload_pack? - if action_name == 'info_refs' - params[:service] == 'git-upload-pack' - else - action_name == 'git_upload_pack' - end + rpc == 'git-upload-pack' end + def receive_pack? + rpc == 'git-receive-pack' + end + + def rpc + if action_name == 'info_refs' + params[:service] + else + action_name.gsub('_', '-') + end + end + def render_ok render json: { 'GL_ID' => Gitlab::ShellEnv.gl_id(@user), @@ -164,4 +174,16 @@ class Projects::GitHttpController < Projects::ApplicationController false end end + + def receive_pack_allowed? + if !Gitlab.config.gitlab_shell.receive_pack + false + elsif user + # Skip user authorization on upload request. + # It will be done by the pre-receive hook in the repository. + true + else + false + end + end end diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index c1aad48ad04..1fa14cadc0f 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -61,12 +61,38 @@ describe 'Git HTTP requests', lib: true do project.update_attribute(:visibility_level, Project::PUBLIC) end - it "responds with status 200" do + it "downloads get status 200" do download(path, env) do |response| expect(response.status).to eq(200) end end + it "uploads get status 401" do + upload(path, env) do |response| + expect(response.status).to eq(401) + end + end + + context "with correct credentials" do + let(:env) { { user: user.username, password: user.password } } + + it "uploads get status 200 (because Git hooks do the real check)" do + upload(path, env) do |response| + expect(response.status).to eq(200) + end + end + + context 'but git-receive-pack is disabled' do + it "responds with status 404" do + allow(Gitlab.config.gitlab_shell).to receive(:receive_pack).and_return(false) + + upload(path, env) do |response| + expect(response.status).to eq(404) + end + end + end + end + context 'but git-upload-pack is disabled' do it "responds with status 404" do allow(Gitlab.config.gitlab_shell).to receive(:upload_pack).and_return(false) @@ -133,13 +159,19 @@ describe 'Git HTTP requests', lib: true do end context "when the user isn't blocked" do - it "responds with status 200" do + it "downloads status 200" do expect(Rack::Attack::Allow2Ban).to receive(:reset) clone_get(path, env) expect(response.status).to eq(200) end + + it "uploads get status 200" do + upload(path, env) do |response| + expect(response.status).to eq(200) + end + end end context "when blank password attempts follow a valid login" do @@ -174,11 +206,17 @@ describe 'Git HTTP requests', lib: true do end context "when the user doesn't have access to the project" do - it "responds with status 404" do + it "downloads get status 404" do download(path, user: user.username, password: user.password) do |response| expect(response.status).to eq(404) end end + + it "uploads get status 200 (because Git hooks do the real check)" do + upload(path, user: user.username, password: user.password) do |response| + expect(response.status).to eq(200) + end + end end end end @@ -196,6 +234,7 @@ describe 'Git HTTP requests', lib: true do end end end + def clone_get(project, options={}) get "/#{project}/info/refs", { service: 'git-upload-pack' }, auth_env(*options.values_at(:user, :password)) end @@ -204,6 +243,14 @@ describe 'Git HTTP requests', lib: true do post "/#{project}/git-upload-pack", {}, auth_env(*options.values_at(:user, :password)) end + def push_get(project, options={}) + get "/#{project}/info/refs", { service: 'git-receive-pack' }, auth_env(*options.values_at(:user, :password)) + end + + def push_post(project, options={}) + post "/#{project}/git-receive-pack", {}, auth_env(*options.values_at(:user, :password)) + end + def download(project, user: nil, password: nil) args = [project, {user: user, password: password}] @@ -214,6 +261,16 @@ describe 'Git HTTP requests', lib: true do yield response end + def upload(project, user: nil, password: nil) + args = [project, {user: user, password: password}] + + push_get *args + yield response + + push_post *args + yield response + end + def auth_env(user, password) if user && password { 'HTTP_AUTHORIZATION' => ActionController::HttpAuthentication::Basic.encode_credentials(user, password) }