From 31bc876b7b34fa1785be022e9cffdc601f2192d7 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Thu, 24 Mar 2016 16:14:09 +0100 Subject: [PATCH] Test both GET and POST for git-upload-pack --- config/routes.rb | 4 +- spec/requests/git_http_spec.rb | 112 ++++++++++++++++++++------------- 2 files changed, 69 insertions(+), 47 deletions(-) diff --git a/config/routes.rb b/config/routes.rb index 021eab89c2a..eace7516e91 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -429,8 +429,8 @@ Rails.application.routes.draw do # Git HTTP clients ('git clone' etc.) scope constraints: { format: /(git|wiki\.git)/ } do get '/info/refs', to: 'git_http#info_refs', only: :get - get '/git-upload-pack', to: 'git_http#git_upload_pack', only: :post - get '/git-receive-pack', to: 'git_http#git_receive_pack', only: :post + post '/git-upload-pack', to: 'git_http#git_upload_pack', only: :post + post '/git-receive-pack', to: 'git_http#git_receive_pack', only: :post end # Blob routes: diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index 7e274b4209b..ef0b83fd475 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -8,26 +8,26 @@ describe 'Git HTTP requests', lib: true do context "when the project doesn't exist" do context "when no authentication is provided" do it "responds with status 401" do - clone_get '/doesnt/exist.git/info/refs' - - expect(response.status).to eq(401) + download('doesnt/exist.git') do |response| + expect(response.status).to eq(401) + end end end context "when username and password are provided" do context "when authentication fails" do it "responds with status 401" do - clone_get '/doesnt/exist.git/info/refs', user: user.username, password: "nope" - - expect(response.status).to eq(401) + download('doesnt/exist.git', user: user.username, password: "nope") do |response| + expect(response.status).to eq(401) + end end end context "when authentication succeeds" do it "responds with status 404" do - clone_get '/doesnt/exist.git/info/refs', user: user.username, password: user.password - - expect(response.status).to eq(404) + download('/doesnt/exist.git', user: user.username, password: user.password) do |response| + expect(response.status).to eq(404) + end end end end @@ -38,23 +38,25 @@ describe 'Git HTTP requests', lib: true do wiki = ProjectWiki.new(project) project.update_attribute(:visibility_level, Project::PUBLIC) - clone_get "/#{wiki.repository.path_with_namespace}.git/info/refs" - json_body = ActiveSupport::JSON.decode(response.body) - - expect(response.status).to eq(200) - expect(json_body['RepoPath']).to include(wiki.repository.path_with_namespace) + download("/#{wiki.repository.path_with_namespace}.git") do |response| + json_body = ActiveSupport::JSON.decode(response.body) + + expect(response.status).to eq(200) + expect(json_body['RepoPath']).to include(wiki.repository.path_with_namespace) + end end end context "when the project exists" do - let(:path) { clone_path(project) } + let(:path) { "#{project.path_with_namespace}.git" } + let(:env) { {} } context "when the project is public" do it "responds with status 200" do project.update_attribute(:visibility_level, Project::PUBLIC) - clone_get path - - expect(response.status).to eq(200) + download(path, env) do |response| + expect(response.status).to eq(200) + end end end @@ -65,33 +67,37 @@ describe 'Git HTTP requests', lib: true do context "when no authentication is provided" do it "responds with status 401" do - clone_get path - - expect(response.status).to eq(401) + download(path, env) do |response| + expect(response.status).to eq(401) + end end end context "when username and password are provided" do + let(:env) { { user: user.username, password: 'nope' } } + context "when authentication fails" do it "responds with status 401" do - clone_get path, user: user.username, password: 'nope' - - expect(response.status).to eq(401) + download(path, env) do |response| + expect(response.status).to eq(401) + end end context "when the user is IP banned" do it "responds with status 401" do expect(Rack::Attack::Allow2Ban).to receive(:filter).and_return(true) allow_any_instance_of(Rack::Request).to receive(:ip).and_return('1.2.3.4') - - clone_get path, user: user.username, password: 'nope' - + + clone_get(path, env) + expect(response.status).to eq(401) end end end context "when authentication succeeds" do + let(:env) { { user: user.username, password: user.password } } + context "when the user has access to the project" do before do project.team << [user, :master] @@ -102,18 +108,18 @@ describe 'Git HTTP requests', lib: true do user.block project.team << [user, :master] - clone_get path, user: user.username, password: user.password - - expect(response.status).to eq(404) + download(path, env) do |response| + expect(response.status).to eq(404) + end end end context "when the user isn't blocked" do it "responds with status 200" do expect(Rack::Attack::Allow2Ban).to receive(:reset) - - clone_get path, user: user.username, password: user.password - + + clone_get(path, env) + expect(response.status).to eq(200) end end @@ -151,9 +157,9 @@ describe 'Git HTTP requests', lib: true do context "when the user doesn't have access to the project" do it "responds with status 404" do - clone_get path, user: user.username, password: user.password - - expect(response.status).to eq(404) + download(path, user: user.username, password: user.password) do |response| + expect(response.status).to eq(404) + end end end end @@ -165,7 +171,7 @@ describe 'Git HTTP requests', lib: true do project = FactoryGirl.create :empty_project project.update_attributes(runners_token: token, builds_enabled: true) - clone_get clone_path(project), user: 'gitlab-ci-token', password: token + clone_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: token expect(response.status).to eq(200) end @@ -174,17 +180,33 @@ describe 'Git HTTP requests', lib: true do end end - def clone_get(url, user: nil, password: nil) - if user && password - env = { 'HTTP_AUTHORIZATION' => ActionController::HttpAuthentication::Basic.encode_credentials(user, password) } - else - env = {} - end - - get url, { 'service' => 'git-upload-pack' }, env + def clone_get(project, options={}) + get "/#{project}/info/refs", { service: 'git-upload-pack' }, auth_env(*options.values_at(:user, :password)) + end + + def clone_post(project, options={}) + post "/#{project}/git-upload-pack", {}, auth_env(*options.values_at(:user, :password)) end def clone_path(project) "/#{project.path_with_namespace}.git/info/refs" end + + def download(project, user: nil, password: nil) + args = [project, {user: user, password: password}] + + clone_get *args + yield response + + clone_post *args + yield response + end + + def auth_env(user, password) + if user && password + { 'HTTP_AUTHORIZATION' => ActionController::HttpAuthentication::Basic.encode_credentials(user, password) } + else + {} + end + end end