From 86f39fece2273319e2c960d96b131924a9fbdc3c Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 15 Jul 2016 14:01:34 +0200 Subject: [PATCH 1/3] Refactor LFS specs to use requests instead of LfsRouter --- spec/lib/gitlab/lfs/lfs_router_spec.rb | 730 ----------------------- spec/requests/lfs_http_spec.rb | 763 +++++++++++++++++++++++++ 2 files changed, 763 insertions(+), 730 deletions(-) delete mode 100644 spec/lib/gitlab/lfs/lfs_router_spec.rb create mode 100644 spec/requests/lfs_http_spec.rb diff --git a/spec/lib/gitlab/lfs/lfs_router_spec.rb b/spec/lib/gitlab/lfs/lfs_router_spec.rb deleted file mode 100644 index 659facd6c19..00000000000 --- a/spec/lib/gitlab/lfs/lfs_router_spec.rb +++ /dev/null @@ -1,730 +0,0 @@ -require 'spec_helper' - -describe Gitlab::Lfs::Router, lib: true do - let(:project) { create(:project) } - let(:public_project) { create(:project, :public) } - let(:forked_project) { fork_project(public_project, user) } - - let(:user) { create(:user) } - let(:user_two) { create(:user) } - let!(:lfs_object) { create(:lfs_object, :with_file) } - - let(:request) { Rack::Request.new(env) } - let(:env) do - { - 'rack.input' => '', - 'REQUEST_METHOD' => 'GET', - } - end - - let(:lfs_router_auth) { new_lfs_router(project, user: user) } - let(:lfs_router_ci_auth) { new_lfs_router(project, ci: true) } - let(:lfs_router_noauth) { new_lfs_router(project) } - let(:lfs_router_public_auth) { new_lfs_router(public_project, user: user) } - let(:lfs_router_public_ci_auth) { new_lfs_router(public_project, ci: true) } - let(:lfs_router_public_noauth) { new_lfs_router(public_project) } - let(:lfs_router_forked_noauth) { new_lfs_router(forked_project) } - let(:lfs_router_forked_auth) { new_lfs_router(forked_project, user: user_two) } - let(:lfs_router_forked_ci_auth) { new_lfs_router(forked_project, ci: true) } - - let(:sample_oid) { "b68143e6463773b1b6c6fd009a76c32aeec041faff32ba2ed42fd7f708a17f80" } - let(:sample_size) { 499013 } - let(:respond_with_deprecated) {[ 501, { "Content-Type" => "application/json; charset=utf-8" }, ["{\"message\":\"Server supports batch API only, please update your Git LFS client to version 1.0.1 and up.\",\"documentation_url\":\"#{Gitlab.config.gitlab.url}/help\"}"]]} - let(:respond_with_disabled) {[ 501, { "Content-Type" => "application/json; charset=utf-8" }, ["{\"message\":\"Git LFS is not enabled on this GitLab server, contact your admin.\",\"documentation_url\":\"#{Gitlab.config.gitlab.url}/help\"}"]]} - - describe 'when lfs is disabled' do - before do - allow(Gitlab.config.lfs).to receive(:enabled).and_return(false) - env['REQUEST_METHOD'] = 'POST' - body = { - 'objects' => [ - { 'oid' => '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897', - 'size' => 1575078 - }, - { 'oid' => sample_oid, - 'size' => sample_size - } - ], - 'operation' => 'upload' - }.to_json - env['rack.input'] = StringIO.new(body) - env["PATH_INFO"] = "#{project.repository.path_with_namespace}.git/info/lfs/objects/batch" - end - - it 'responds with 501' do - expect(lfs_router_auth.try_call).to match_array(respond_with_disabled) - end - end - - describe 'when fetching lfs object using deprecated API' do - before do - enable_lfs - env["PATH_INFO"] = "#{project.repository.path_with_namespace}.git/info/lfs/objects/#{sample_oid}" - end - - it 'responds with 501' do - expect(lfs_router_auth.try_call).to match_array(respond_with_deprecated) - end - end - - describe 'when fetching lfs object' do - before do - enable_lfs - env['HTTP_ACCEPT'] = "application/vnd.git-lfs+json; charset=utf-8" - env["PATH_INFO"] = "#{project.repository.path_with_namespace}.git/gitlab-lfs/objects/#{sample_oid}" - end - - describe 'and request comes from gitlab-workhorse' do - context 'without user being authorized' do - it "responds with status 401" do - expect(lfs_router_noauth.try_call.first).to eq(401) - end - end - - context 'with required headers' do - before do - project.lfs_objects << lfs_object - env['HTTP_X_SENDFILE_TYPE'] = "X-Sendfile" - end - - context 'when user does not have project access' do - it "responds with status 403" do - expect(lfs_router_auth.try_call.first).to eq(403) - end - end - - context 'when user has project access' do - before do - project.team << [user, :master] - end - - it "responds with status 200" do - expect(lfs_router_auth.try_call.first).to eq(200) - end - - it "responds with the file location" do - expect(lfs_router_auth.try_call[1]['Content-Type']).to eq("application/octet-stream") - expect(lfs_router_auth.try_call[1]['X-Sendfile']).to eq(lfs_object.file.path) - end - end - - context 'when CI is authorized' do - it "responds with status 200" do - expect(lfs_router_ci_auth.try_call.first).to eq(200) - end - - it "responds with the file location" do - expect(lfs_router_ci_auth.try_call[1]['Content-Type']).to eq("application/octet-stream") - expect(lfs_router_ci_auth.try_call[1]['X-Sendfile']).to eq(lfs_object.file.path) - end - end - end - - context 'without required headers' do - it "responds with status 403" do - expect(lfs_router_auth.try_call.first).to eq(403) - end - end - end - end - - describe 'when handling lfs request using deprecated API' do - before do - enable_lfs - env['REQUEST_METHOD'] = 'POST' - env["PATH_INFO"] = "#{project.repository.path_with_namespace}.git/info/lfs/objects" - end - - it 'responds with 501' do - expect(lfs_router_auth.try_call).to match_array(respond_with_deprecated) - end - end - - describe 'when handling lfs batch request' do - before do - enable_lfs - env['REQUEST_METHOD'] = 'POST' - env['PATH_INFO'] = "#{project.repository.path_with_namespace}.git/info/lfs/objects/batch" - end - - describe 'download' do - before do - body = { 'operation' => 'download', - 'objects' => [ - { 'oid' => sample_oid, - 'size' => sample_size - }] - }.to_json - env['rack.input'] = StringIO.new(body) - end - - shared_examples 'an authorized requests' do - context 'when downloading an lfs object that is assigned to our project' do - before do - project.lfs_objects << lfs_object - end - - it 'responds with status 200 and href to download' do - response = router.try_call - expect(response.first).to eq(200) - response_body = ActiveSupport::JSON.decode(response.last.first) - - expect(response_body).to eq('objects' => [ - { 'oid' => sample_oid, - 'size' => sample_size, - 'actions' => { - 'download' => { - 'href' => "#{project.http_url_to_repo}/gitlab-lfs/objects/#{sample_oid}", - 'header' => { 'Authorization' => auth } - } - } - }]) - end - end - - context 'when downloading an lfs object that is assigned to other project' do - before do - public_project.lfs_objects << lfs_object - end - - it 'responds with status 200 and error message' do - response = router.try_call - expect(response.first).to eq(200) - response_body = ActiveSupport::JSON.decode(response.last.first) - - expect(response_body).to eq('objects' => [ - { 'oid' => sample_oid, - 'size' => sample_size, - 'error' => { - 'code' => 404, - 'message' => "Object does not exist on the server or you don't have permissions to access it", - } - }]) - end - end - - context 'when downloading a lfs object that does not exist' do - before do - body = { 'operation' => 'download', - 'objects' => [ - { 'oid' => '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897', - 'size' => 1575078 - }] - }.to_json - env['rack.input'] = StringIO.new(body) - end - - it "responds with status 200 and error message" do - response = router.try_call - expect(response.first).to eq(200) - response_body = ActiveSupport::JSON.decode(response.last.first) - - expect(response_body).to eq('objects' => [ - { 'oid' => '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897', - 'size' => 1575078, - 'error' => { - 'code' => 404, - 'message' => "Object does not exist on the server or you don't have permissions to access it", - } - }]) - end - end - - context 'when downloading one new and one existing lfs object' do - before do - body = { 'operation' => 'download', - 'objects' => [ - { 'oid' => '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897', - 'size' => 1575078 - }, - { 'oid' => sample_oid, - 'size' => sample_size - } - ] - }.to_json - env['rack.input'] = StringIO.new(body) - project.lfs_objects << lfs_object - end - - it "responds with status 200 with upload hypermedia link for the new object" do - response = router.try_call - expect(response.first).to eq(200) - response_body = ActiveSupport::JSON.decode(response.last.first) - - expect(response_body).to eq('objects' => [ - { 'oid' => '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897', - 'size' => 1575078, - 'error' => { - 'code' => 404, - 'message' => "Object does not exist on the server or you don't have permissions to access it", - } - }, - { 'oid' => sample_oid, - 'size' => sample_size, - 'actions' => { - 'download' => { - 'href' => "#{project.http_url_to_repo}/gitlab-lfs/objects/#{sample_oid}", - 'header' => { 'Authorization' => auth } - } - } - }]) - end - end - end - - context 'when user is authenticated' do - let(:auth) { authorize(user) } - - before do - env["HTTP_AUTHORIZATION"] = auth - project.team << [user, role] - end - - it_behaves_like 'an authorized requests' do - let(:role) { :reporter } - let(:router) { lfs_router_auth } - end - - context 'when user does is not member of the project' do - let(:role) { :guest } - - it 'responds with 403' do - expect(lfs_router_auth.try_call.first).to eq(403) - end - end - - context 'when user does not have download access' do - let(:role) { :guest } - - it 'responds with 403' do - expect(lfs_router_auth.try_call.first).to eq(403) - end - end - end - - context 'when CI is authorized' do - let(:auth) { 'gitlab-ci-token:password' } - - before do - env["HTTP_AUTHORIZATION"] = auth - end - - it_behaves_like 'an authorized requests' do - let(:router) { lfs_router_ci_auth } - end - end - - context 'when user is not authenticated' do - describe 'is accessing public project' do - before do - public_project.lfs_objects << lfs_object - end - - it 'responds with status 200 and href to download' do - response = lfs_router_public_noauth.try_call - expect(response.first).to eq(200) - response_body = ActiveSupport::JSON.decode(response.last.first) - - expect(response_body).to eq('objects' => [ - { 'oid' => sample_oid, - 'size' => sample_size, - 'actions' => { - 'download' => { - 'href' => "#{public_project.http_url_to_repo}/gitlab-lfs/objects/#{sample_oid}", - 'header' => {} - } - } - }]) - end - end - - describe 'is accessing non-public project' do - before do - project.lfs_objects << lfs_object - end - - it 'responds with authorization required' do - expect(lfs_router_noauth.try_call.first).to eq(401) - end - end - end - end - - describe 'upload' do - before do - body = { 'operation' => 'upload', - 'objects' => [ - { 'oid' => sample_oid, - 'size' => sample_size - }] - }.to_json - env['rack.input'] = StringIO.new(body) - end - - describe 'when request is authenticated' do - describe 'when user has project push access' do - before do - @auth = authorize(user) - env["HTTP_AUTHORIZATION"] = @auth - project.team << [user, :developer] - end - - context 'when pushing an lfs object that already exists' do - before do - public_project.lfs_objects << lfs_object - end - - it "responds with status 200 and links the object to the project" do - response_body = lfs_router_auth.try_call.last - response = ActiveSupport::JSON.decode(response_body.first) - - expect(response['objects']).to be_kind_of(Array) - expect(response['objects'].first['oid']).to eq(sample_oid) - expect(response['objects'].first['size']).to eq(sample_size) - expect(lfs_object.projects.pluck(:id)).not_to include(project.id) - expect(lfs_object.projects.pluck(:id)).to include(public_project.id) - expect(response['objects'].first['actions']['upload']['href']).to eq("#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}.git/gitlab-lfs/objects/#{sample_oid}/#{sample_size}") - expect(response['objects'].first['actions']['upload']['header']).to eq('Authorization' => @auth) - end - end - - context 'when pushing a lfs object that does not exist' do - before do - body = { 'operation' => 'upload', - 'objects' => [ - { 'oid' => '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897', - 'size' => 1575078 - }] - }.to_json - env['rack.input'] = StringIO.new(body) - end - - it "responds with status 200 and upload hypermedia link" do - response = lfs_router_auth.try_call - expect(response.first).to eq(200) - - response_body = ActiveSupport::JSON.decode(response.last.first) - expect(response_body['objects']).to be_kind_of(Array) - expect(response_body['objects'].first['oid']).to eq("91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897") - expect(response_body['objects'].first['size']).to eq(1575078) - expect(lfs_object.projects.pluck(:id)).not_to include(project.id) - expect(response_body['objects'].first['actions']['upload']['href']).to eq("#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}.git/gitlab-lfs/objects/91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897/1575078") - expect(response_body['objects'].first['actions']['upload']['header']).to eq('Authorization' => @auth) - end - end - - context 'when pushing one new and one existing lfs object' do - before do - body = { 'operation' => 'upload', - 'objects' => [ - { 'oid' => '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897', - 'size' => 1575078 - }, - { 'oid' => sample_oid, - 'size' => sample_size - } - ] - }.to_json - env['rack.input'] = StringIO.new(body) - project.lfs_objects << lfs_object - end - - it "responds with status 200 with upload hypermedia link for the new object" do - response = lfs_router_auth.try_call - expect(response.first).to eq(200) - - response_body = ActiveSupport::JSON.decode(response.last.first) - expect(response_body['objects']).to be_kind_of(Array) - - expect(response_body['objects'].first['oid']).to eq("91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897") - expect(response_body['objects'].first['size']).to eq(1575078) - expect(response_body['objects'].first['actions']['upload']['href']).to eq("#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}.git/gitlab-lfs/objects/91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897/1575078") - expect(response_body['objects'].first['actions']['upload']['header']).to eq("Authorization" => @auth) - - expect(response_body['objects'].last['oid']).to eq(sample_oid) - expect(response_body['objects'].last['size']).to eq(sample_size) - expect(response_body['objects'].last).not_to have_key('actions') - end - end - end - - context 'when user does not have push access' do - it 'responds with 403' do - expect(lfs_router_auth.try_call.first).to eq(403) - end - end - - context 'when CI is authorized' do - it 'responds with 401' do - expect(lfs_router_ci_auth.try_call.first).to eq(401) - end - end - end - - context 'when user is not authenticated' do - context 'when user has push access' do - before do - project.team << [user, :master] - end - - it "responds with status 401" do - expect(lfs_router_public_noauth.try_call.first).to eq(401) - end - end - - context 'when user does not have push access' do - it "responds with status 401" do - expect(lfs_router_public_noauth.try_call.first).to eq(401) - end - end - end - - context 'when CI is authorized' do - let(:auth) { 'gitlab-ci-token:password' } - - before do - env["HTTP_AUTHORIZATION"] = auth - end - - it "responds with status 403" do - expect(lfs_router_public_ci_auth.try_call.first).to eq(401) - end - end - end - - describe 'unsupported' do - before do - body = { 'operation' => 'other', - 'objects' => [ - { 'oid' => sample_oid, - 'size' => sample_size - }] - }.to_json - env['rack.input'] = StringIO.new(body) - end - - it 'responds with status 404' do - expect(lfs_router_public_noauth.try_call.first).to eq(404) - end - end - end - - describe 'when pushing a lfs object' do - before do - enable_lfs - env['REQUEST_METHOD'] = 'PUT' - end - - shared_examples 'unauthorized' do - context 'and request is sent by gitlab-workhorse to authorize the request' do - before do - header_for_upload_authorize(router.project) - end - - it 'responds with status 401' do - expect(router.try_call.first).to eq(401) - end - end - - context 'and request is sent by gitlab-workhorse to finalize the upload' do - before do - headers_for_upload_finalize(router.project) - end - - it 'responds with status 401' do - expect(router.try_call.first).to eq(401) - end - end - - context 'and request is sent with a malformed headers' do - before do - env["PATH_INFO"] = "#{router.project.repository.path_with_namespace}.git/gitlab-lfs/objects/#{sample_oid}/#{sample_size}" - env["HTTP_X_GITLAB_LFS_TMP"] = "cat /etc/passwd" - end - - it 'does not recognize it as a valid lfs command' do - expect(router.try_call).to eq(nil) - end - end - end - - shared_examples 'forbidden' do - context 'and request is sent by gitlab-workhorse to authorize the request' do - before do - header_for_upload_authorize(router.project) - end - - it 'responds with 403' do - expect(router.try_call.first).to eq(403) - end - end - - context 'and request is sent by gitlab-workhorse to finalize the upload' do - before do - headers_for_upload_finalize(router.project) - end - - it 'responds with 403' do - expect(router.try_call.first).to eq(403) - end - end - end - - describe 'to one project' do - describe 'when user is authenticated' do - describe 'when user has push access to the project' do - before do - project.team << [user, :developer] - end - - context 'and request is sent by gitlab-workhorse to authorize the request' do - before do - header_for_upload_authorize(project) - end - - it 'responds with status 200, location of lfs store and object details' do - json_response = ActiveSupport::JSON.decode(lfs_router_auth.try_call.last.first) - - expect(lfs_router_auth.try_call.first).to eq(200) - expect(json_response['StoreLFSPath']).to eq("#{Gitlab.config.shared.path}/lfs-objects/tmp/upload") - expect(json_response['LfsOid']).to eq(sample_oid) - expect(json_response['LfsSize']).to eq(sample_size) - end - end - - context 'and request is sent by gitlab-workhorse to finalize the upload' do - before do - headers_for_upload_finalize(project) - end - - it 'responds with status 200 and lfs object is linked to the project' do - expect(lfs_router_auth.try_call.first).to eq(200) - expect(lfs_object.projects.pluck(:id)).to include(project.id) - end - end - end - - describe 'and user does not have push access' do - let(:router) { lfs_router_auth } - - it_behaves_like 'forbidden' - end - end - - context 'when CI is authenticated' do - let(:router) { lfs_router_ci_auth } - - it_behaves_like 'unauthorized' - end - - context 'for unauthenticated' do - let(:router) { new_lfs_router(project) } - - it_behaves_like 'unauthorized' - end - end - - describe 'to a forked project' do - let(:forked_project) { fork_project(public_project, user) } - - describe 'when user is authenticated' do - describe 'when user has push access to the project' do - before do - forked_project.team << [user_two, :developer] - end - - context 'and request is sent by gitlab-workhorse to authorize the request' do - before do - header_for_upload_authorize(forked_project) - end - - it 'responds with status 200, location of lfs store and object details' do - json_response = ActiveSupport::JSON.decode(lfs_router_forked_auth.try_call.last.first) - - expect(lfs_router_forked_auth.try_call.first).to eq(200) - expect(json_response['StoreLFSPath']).to eq("#{Gitlab.config.shared.path}/lfs-objects/tmp/upload") - expect(json_response['LfsOid']).to eq(sample_oid) - expect(json_response['LfsSize']).to eq(sample_size) - end - end - - context 'and request is sent by gitlab-workhorse to finalize the upload' do - before do - headers_for_upload_finalize(forked_project) - end - - it 'responds with status 200 and lfs object is linked to the source project' do - expect(lfs_router_forked_auth.try_call.first).to eq(200) - expect(lfs_object.projects.pluck(:id)).to include(public_project.id) - end - end - end - - describe 'and user does not have push access' do - let(:router) { lfs_router_forked_auth } - - it_behaves_like 'forbidden' - end - end - - context 'when CI is authenticated' do - let(:router) { lfs_router_forked_ci_auth } - - it_behaves_like 'unauthorized' - end - - context 'for unauthenticated' do - let(:router) { lfs_router_forked_noauth } - - it_behaves_like 'unauthorized' - end - - describe 'and second project not related to fork or a source project' do - let(:second_project) { create(:project) } - let(:lfs_router_second_project) { new_lfs_router(second_project, user: user) } - - before do - public_project.lfs_objects << lfs_object - headers_for_upload_finalize(second_project) - end - - context 'when pushing the same lfs object to the second project' do - before do - second_project.team << [user, :master] - end - - it 'responds with 200 and links the lfs object to the project' do - expect(lfs_router_second_project.try_call.first).to eq(200) - expect(lfs_object.projects.pluck(:id)).to include(second_project.id, public_project.id) - end - end - end - end - end - - def enable_lfs - allow(Gitlab.config.lfs).to receive(:enabled).and_return(true) - end - - def authorize(user) - ActionController::HttpAuthentication::Basic.encode_credentials(user.username, user.password) - end - - def new_lfs_router(project, user: nil, ci: false) - Gitlab::Lfs::Router.new(project, user, ci, request) - end - - def header_for_upload_authorize(project) - env["PATH_INFO"] = "#{project.repository.path_with_namespace}.git/gitlab-lfs/objects/#{sample_oid}/#{sample_size}/authorize" - end - - def headers_for_upload_finalize(project) - env["PATH_INFO"] = "#{project.repository.path_with_namespace}.git/gitlab-lfs/objects/#{sample_oid}/#{sample_size}" - env["HTTP_X_GITLAB_LFS_TMP"] = "#{sample_oid}6e561c9d4" - end - - def fork_project(project, user, object = nil) - allow(RepositoryForkWorker).to receive(:perform_async).and_return(true) - Projects::ForkService.new(project, user, {}).execute - end -end diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb new file mode 100644 index 00000000000..d862cb5b0b4 --- /dev/null +++ b/spec/requests/lfs_http_spec.rb @@ -0,0 +1,763 @@ +require 'spec_helper' + +describe Gitlab::Lfs::Router do + let(:user) { create(:user) } + let!(:lfs_object) { create(:lfs_object, :with_file) } + + let(:headers) do + { + 'Authorization' => authorization, + 'X-Sendfile-Type' => sendfile + }.compact + end + let(:authorization) { } + let(:sendfile) { } + + let(:sample_oid) { lfs_object.oid } + let(:sample_size) { lfs_object.size } + + describe 'when lfs is disabled' do + let(:project) { create(:empty_project) } + let(:body) do + { + 'objects' => [ + { 'oid' => '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897', + 'size' => 1575078 + }, + { 'oid' => sample_oid, + 'size' => sample_size + } + ], + 'operation' => 'upload' + } + end + + before do + allow(Gitlab.config.lfs).to receive(:enabled).and_return(false) + post_json "#{project.http_url_to_repo}/info/lfs/objects/batch", body, headers + end + + it 'responds with 501' do + expect(response).to have_http_status(501) + expect(json_response).to include('message' => 'Git LFS is not enabled on this GitLab server, contact your admin.') + end + end + + describe 'deprecated API' do + let(:project) { create(:empty_project) } + + before do + enable_lfs + end + + shared_examples 'a deprecated' do + it 'responds with 501' do + expect(response).to have_http_status(501) + end + + it 'returns deprecated message' do + expect(json_response).to include('message' => 'Server supports batch API only, please update your Git LFS client to version 1.0.1 and up.') + end + end + + context 'when fetching lfs object using deprecated API' do + let(:authorization) { authorize_user } + + before do + get "#{project.http_url_to_repo}/info/lfs/objects/#{sample_oid}", nil, headers + end + + it_behaves_like 'a deprecated' + end + + context 'when handling lfs request using deprecated API' do + before do + post_json "#{project.http_url_to_repo}/info/lfs/objects", nil, headers + end + + it_behaves_like 'a deprecated' + end + end + + describe 'when fetching lfs object' do + let(:project) { create(:empty_project) } + let(:update_permissions) { } + + before do + enable_lfs + update_permissions + get "#{project.http_url_to_repo}/gitlab-lfs/objects/#{sample_oid}", nil, headers + end + + context 'and request comes from gitlab-workhorse' do + context 'without user being authorized' do + it 'responds with status 401' do + expect(response).to have_http_status(401) + end + end + + context 'with required headers' do + shared_examples 'responds with a file' do + it 'responds with status 200' do + expect(response).to have_http_status(200) + end + + it 'responds with the file location' do + expect(response.headers['Content-Type']).to eq('application/octet-stream') + expect(response.headers['X-Sendfile']).to eq(lfs_object.file.path) + end + end + + context 'with user is authorized' do + let(:authorization) { authorize_user } + let(:sendfile) { 'X-Sendfile' } + + context 'and does not have project access' do + let(:update_permissions) do + project.lfs_objects << lfs_object + end + + it 'responds with status 403' do + expect(response).to have_http_status(403) + end + end + + context 'and does have project access' do + let(:update_permissions) do + project.team << [user, :master] + project.lfs_objects << lfs_object + end + + it_behaves_like 'responds with a file' + end + end + + context 'when CI is authorized' do + let(:authorization) { authorize_ci_project } + + it_behaves_like 'responds with a file' + end + end + + context 'without required headers' do + let(:authorization) { authorize_user } + + it 'responds with status 403' do + expect(response).to have_http_status(403) + end + end + end + end + + describe 'when handling lfs batch request' do + let(:update_lfs_permissions) { } + let(:update_user_permissions) { } + + before do + enable_lfs + update_lfs_permissions + update_user_permissions + post_json "#{project.http_url_to_repo}/info/lfs/objects/batch", body, headers + end + + describe 'download' do + let(:project) { create(:empty_project) } + let(:body) do + { 'operation' => 'download', + 'objects' => [ + { 'oid' => sample_oid, + 'size' => sample_size + }] + } + end + + shared_examples 'an authorized requests' do + context 'when downloading an lfs object that is assigned to our project' do + let(:update_lfs_permissions) do + project.lfs_objects << lfs_object + end + + it 'responds with status 200' do + expect(response).to have_http_status(200) + end + + it 'with href to download' do + expect(json_response).to eq('objects' => [ + { 'oid' => sample_oid, + 'size' => sample_size, + 'actions' => { + 'download' => { + 'href' => "#{project.http_url_to_repo}/gitlab-lfs/objects/#{sample_oid}", + 'header' => { 'Authorization' => authorization } + } + } + }]) + end + end + + context 'when downloading an lfs object that is assigned to other project' do + let(:other_project) { create(:empty_project) } + let(:update_lfs_permissions) do + other_project.lfs_objects << lfs_object + end + + it 'responds with status 200' do + expect(response).to have_http_status(200) + end + + it 'with href to download' do + expect(json_response).to eq('objects' => [ + { 'oid' => sample_oid, + 'size' => sample_size, + 'error' => { + 'code' => 404, + 'message' => "Object does not exist on the server or you don't have permissions to access it", + } + }]) + end + end + + context 'when downloading a lfs object that does not exist' do + let(:body) do + { 'operation' => 'download', + 'objects' => [ + { 'oid' => '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897', + 'size' => 1575078 + }] + } + end + + it 'responds with status 200' do + expect(response).to have_http_status(200) + end + + it 'with an 404 for specific object' do + expect(json_response).to eq('objects' => [ + { 'oid' => '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897', + 'size' => 1575078, + 'error' => { + 'code' => 404, + 'message' => "Object does not exist on the server or you don't have permissions to access it", + } + }]) + end + end + + context 'when downloading one new and one existing lfs object' do + let(:body) do + { 'operation' => 'download', + 'objects' => [ + { 'oid' => '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897', + 'size' => 1575078 + }, + { 'oid' => sample_oid, + 'size' => sample_size + } + ] + } + end + + let(:update_lfs_permissions) do + project.lfs_objects << lfs_object + end + + it 'responds with status 200' do + expect(response).to have_http_status(200) + end + + it 'responds with upload hypermedia link for the new object' do + expect(json_response).to eq('objects' => [ + { 'oid' => '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897', + 'size' => 1575078, + 'error' => { + 'code' => 404, + 'message' => "Object does not exist on the server or you don't have permissions to access it", + } + }, + { 'oid' => sample_oid, + 'size' => sample_size, + 'actions' => { + 'download' => { + 'href' => "#{project.http_url_to_repo}/gitlab-lfs/objects/#{sample_oid}", + 'header' => { 'Authorization' => authorization } + } + } + }]) + end + end + end + + context 'when user is authenticated' do + let(:authorization) { authorize_user } + + let(:update_user_permissions) do + project.team << [user, role] + end + + it_behaves_like 'an authorized requests' do + let(:role) { :reporter } + end + + context 'when user does is not member of the project' do + let(:role) { :guest } + + it 'responds with 403' do + expect(response).to have_http_status(403) + end + end + + context 'when user does not have download access' do + let(:role) { :guest } + + it 'responds with 403' do + expect(response).to have_http_status(403) + end + end + end + + context 'when CI is authorized' do + let(:authorization) { authorize_ci_project } + + it_behaves_like 'an authorized requests' + end + + context 'when user is not authenticated' do + describe 'is accessing public project' do + let(:project) { create(:project, :public) } + + let(:update_lfs_permissions) do + project.lfs_objects << lfs_object + end + + it 'responds with status 200 and href to download' do + expect(response).to have_http_status(200) + end + + it 'responds with status 200 and href to download' do + expect(json_response).to eq('objects' => [ + { 'oid' => sample_oid, + 'size' => sample_size, + 'actions' => { + 'download' => { + 'href' => "#{project.http_url_to_repo}/gitlab-lfs/objects/#{sample_oid}", + 'header' => {} + } + } + }]) + end + end + + describe 'is accessing non-public project' do + let(:update_lfs_permissions) do + project.lfs_objects << lfs_object + end + + it 'responds with authorization required' do + expect(response).to have_http_status(401) + end + end + end + end + + describe 'upload' do + let(:project) { create(:project, :public) } + let(:body) do + { 'operation' => 'upload', + 'objects' => [ + { 'oid' => sample_oid, + 'size' => sample_size + }] + } + end + + describe 'when request is authenticated' do + describe 'when user has project push access' do + let(:authorization) { authorize_user } + + let(:update_user_permissions) do + project.team << [user, :developer] + end + + context 'when pushing an lfs object that already exists' do + let(:other_project) { create(:empty_project) } + let(:update_lfs_permissions) do + other_project.lfs_objects << lfs_object + end + + it 'responds with status 200' do + expect(response).to have_http_status(200) + end + + it 'responds with links the object to the project' do + expect(json_response['objects']).to be_kind_of(Array) + expect(json_response['objects'].first['oid']).to eq(sample_oid) + expect(json_response['objects'].first['size']).to eq(sample_size) + expect(lfs_object.projects.pluck(:id)).not_to include(project.id) + expect(lfs_object.projects.pluck(:id)).to include(other_project.id) + expect(json_response['objects'].first['actions']['upload']['href']).to eq("#{project.http_url_to_repo}/gitlab-lfs/objects/#{sample_oid}/#{sample_size}") + expect(json_response['objects'].first['actions']['upload']['header']).to eq('Authorization' => authorization) + end + end + + context 'when pushing a lfs object that does not exist' do + let(:body) do + { 'operation' => 'upload', + 'objects' => [ + { 'oid' => '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897', + 'size' => 1575078 + }] + } + end + + it 'responds with status 200' do + expect(response).to have_http_status(200) + end + + it 'responds with upload hypermedia link' do + expect(json_response['objects']).to be_kind_of(Array) + expect(json_response['objects'].first['oid']).to eq("91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897") + expect(json_response['objects'].first['size']).to eq(1575078) + expect(json_response['objects'].first['actions']['upload']['href']).to eq("#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}.git/gitlab-lfs/objects/91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897/1575078") + expect(json_response['objects'].first['actions']['upload']['header']).to eq('Authorization' => authorization) + end + end + + context 'when pushing one new and one existing lfs object' do + let(:body) do + { 'operation' => 'upload', + 'objects' => [ + { 'oid' => '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897', + 'size' => 1575078 + }, + { 'oid' => sample_oid, + 'size' => sample_size + } + ] + } + end + + let(:update_lfs_permissions) do + project.lfs_objects << lfs_object + end + + it 'responds with status 200' do + expect(response).to have_http_status(200) + end + + it 'responds with upload hypermedia link for the new object' do + expect(json_response['objects']).to be_kind_of(Array) + + expect(json_response['objects'].first['oid']).to eq("91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897") + expect(json_response['objects'].first['size']).to eq(1575078) + expect(json_response['objects'].first['actions']['upload']['href']).to eq("#{project.http_url_to_repo}/gitlab-lfs/objects/91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897/1575078") + expect(json_response['objects'].first['actions']['upload']['header']).to eq("Authorization" => authorization) + + expect(json_response['objects'].last['oid']).to eq(sample_oid) + expect(json_response['objects'].last['size']).to eq(sample_size) + expect(json_response['objects'].last).not_to have_key('actions') + end + end + end + + context 'when user does not have push access' do + let(:authorization) { authorize_user } + + it 'responds with 403' do + expect(response).to have_http_status(403) + end + end + + context 'when CI is authorized' do + let(:authorization) { authorize_ci_project } + + it 'responds with 401' do + expect(response).to have_http_status(401) + end + end + end + + context 'when user is not authenticated' do + context 'when user has push access' do + let(:update_user_permissions) do + project.team << [user, :master] + end + + it 'responds with status 401' do + expect(response).to have_http_status(401) + end + end + + context 'when user does not have push access' do + it 'responds with status 401' do + expect(response).to have_http_status(401) + end + end + end + + context 'when CI is authorized' do + let(:authorization) { authorize_ci_project } + + it 'responds with status 403' do + expect(response).to have_http_status(401) + end + end + end + + describe 'unsupported' do + let(:project) { create(:empty_project) } + let(:body) do + { 'operation' => 'other', + 'objects' => [ + { 'oid' => sample_oid, + 'size' => sample_size + }] + } + end + + it 'responds with status 404' do + expect(response).to have_http_status(404) + end + end + end + + describe 'when pushing a lfs object' do + before do + enable_lfs + end + + shared_examples 'unauthorized' do + context 'and request is sent by gitlab-workhorse to authorize the request' do + before do + put_authorize + end + + it 'responds with status 401' do + expect(response).to have_http_status(401) + end + end + + context 'and request is sent by gitlab-workhorse to finalize the upload' do + before do + put_finalize + end + + it 'responds with status 401' do + expect(response).to have_http_status(401) + end + end + + context 'and request is sent with a malformed headers' do + before do + put_finalize('cat /etc/passwd') + end + + it 'does not recognize it as a valid lfs command' do + expect(response).to have_http_status(401) + end + end + end + + shared_examples 'forbidden' do + context 'and request is sent by gitlab-workhorse to authorize the request' do + before do + put_authorize + end + + it 'responds with 403' do + expect(response).to have_http_status(403) + end + end + + context 'and request is sent by gitlab-workhorse to finalize the upload' do + before do + put_finalize + end + + it 'responds with 403' do + expect(response).to have_http_status(403) + end + end + end + + describe 'to one project' do + let(:project) { create(:empty_project) } + + describe 'when user is authenticated' do + let(:authorization) { authorize_user } + + describe 'when user has push access to the project' do + before do + project.team << [user, :developer] + end + + context 'and request is sent by gitlab-workhorse to authorize the request' do + before do + put_authorize + end + + it 'responds with status 200' do + expect(response).to have_http_status(200) + end + + it 'responds with status 200, location of lfs store and object details' do + expect(json_response['StoreLFSPath']).to eq("#{Gitlab.config.shared.path}/lfs-objects/tmp/upload") + expect(json_response['LfsOid']).to eq(sample_oid) + expect(json_response['LfsSize']).to eq(sample_size) + end + end + + context 'and request is sent by gitlab-workhorse to finalize the upload' do + before do + put_finalize + end + + it 'responds with status 200' do + expect(response).to have_http_status(200) + end + + it 'lfs object is linked to the project' do + expect(lfs_object.projects.pluck(:id)).to include(project.id) + end + end + end + + describe 'and user does not have push access' do + it_behaves_like 'forbidden' + end + end + + context 'when CI is authenticated' do + let(:authorization) { authorize_ci_project } + + it_behaves_like 'unauthorized' + end + + context 'for unauthenticated' do + it_behaves_like 'unauthorized' + end + end + + describe 'to a forked project' do + let(:upstream_project) { create(:project, :public) } + let(:project_owner) { create(:user) } + let(:project) { fork_project(upstream_project, project_owner) } + + describe 'when user is authenticated' do + let(:authorization) { authorize_user } + + describe 'when user has push access to the project' do + before do + project.team << [user, :developer] + end + + context 'and request is sent by gitlab-workhorse to authorize the request' do + before do + put_authorize + end + + it 'responds with status 200' do + expect(response).to have_http_status(200) + end + + it 'with location of lfs store and object details' do + expect(json_response['StoreLFSPath']).to eq("#{Gitlab.config.shared.path}/lfs-objects/tmp/upload") + expect(json_response['LfsOid']).to eq(sample_oid) + expect(json_response['LfsSize']).to eq(sample_size) + end + end + + context 'and request is sent by gitlab-workhorse to finalize the upload' do + before do + put_finalize + end + + it 'responds with status 200' do + expect(response).to have_http_status(200) + end + + it 'lfs object is linked to the source project' do + expect(lfs_object.projects.pluck(:id)).to include(upstream_project.id) + end + end + end + + describe 'and user does not have push access' do + it_behaves_like 'forbidden' + end + end + + context 'when CI is authenticated' do + let(:authorization) { authorize_ci_project } + + it_behaves_like 'unauthorized' + end + + context 'for unauthenticated' do + it_behaves_like 'unauthorized' + end + + describe 'and second project not related to fork or a source project' do + let(:second_project) { create(:empty_project) } + let(:authorization) { authorize_user } + + before do + second_project.team << [user, :master] + upstream_project.lfs_objects << lfs_object + end + + context 'when pushing the same lfs object to the second project' do + before do + put "#{second_project.http_url_to_repo}/gitlab-lfs/objects/#{sample_oid}/#{sample_size}", nil, + headers.merge('X-Gitlab-Lfs-Tmp' => lfs_tmp_file).compact + end + + it 'responds with status 200' do + expect(response).to have_http_status(200) + end + + it 'links the lfs object to the project' do + expect(lfs_object.projects.pluck(:id)).to include(second_project.id, upstream_project.id) + end + end + end + end + + def put_authorize + put "#{project.http_url_to_repo}/gitlab-lfs/objects/#{sample_oid}/#{sample_size}/authorize", nil, headers + end + + def put_finalize(lfs_tmp = lfs_tmp_file) + put "#{project.http_url_to_repo}/gitlab-lfs/objects/#{sample_oid}/#{sample_size}", nil, + headers.merge('X-Gitlab-Lfs-Tmp' => lfs_tmp).compact + end + + def lfs_tmp_file + "#{sample_oid}012345678" + end + end + + def enable_lfs + allow(Gitlab.config.lfs).to receive(:enabled).and_return(true) + end + + def authorize_ci_project + ActionController::HttpAuthentication::Basic.encode_credentials('gitlab-ci-token', project.runners_token) + end + + def authorize_user + ActionController::HttpAuthentication::Basic.encode_credentials(user.username, user.password) + end + + def fork_project(project, user, object = nil) + allow(RepositoryForkWorker).to receive(:perform_async).and_return(true) + Projects::ForkService.new(project, user, {}).execute + end + + def post_json(url, body = nil, headers = nil) + post(url, body.try(:to_json), (headers || {}).merge('Content-Type' => 'application/json')) + end + + def json_response + @json_response ||= JSON.parse(response.body) + end +end From bb9f827ddc51eba73c78be83b977a07e10638936 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 15 Jul 2016 14:23:26 +0200 Subject: [PATCH 2/3] Fix fetching LFS objects for private CI projects --- CHANGELOG | 1 + lib/gitlab/backend/grack_auth.rb | 2 +- spec/requests/lfs_http_spec.rb | 7 ++++++- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index a977fc3fdbf..671a7c2e4a9 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -18,6 +18,7 @@ v 8.10.0 (unreleased) - Fix MR-auto-close text added to description. !4836 - Fix issue, preventing users w/o push access to sort tags !5105 (redetection) - Add Spring EmojiOne updates. + - Fix fetching LFS objects for private CI projects - Add syntax for multiline blockquote using `>>>` fence !3954 - Fix viewing notification settings when a project is pending deletion - Fix pagination when sorting by columns with lots of ties (like priority) diff --git a/lib/gitlab/backend/grack_auth.rb b/lib/gitlab/backend/grack_auth.rb index 478f145bfed..ab94abeda77 100644 --- a/lib/gitlab/backend/grack_auth.rb +++ b/lib/gitlab/backend/grack_auth.rb @@ -63,7 +63,7 @@ module Grack def ci_request?(login, password) matched_login = /(?^[a-zA-Z]*-ci)-token$/.match(login) - if project && matched_login.present? && git_cmd == 'git-upload-pack' + if project && matched_login.present? underscored_service = matched_login['s'].underscore if underscored_service == 'gitlab_ci' diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index d862cb5b0b4..aaad1e3a6ec 100644 --- a/spec/requests/lfs_http_spec.rb +++ b/spec/requests/lfs_http_spec.rb @@ -98,6 +98,8 @@ describe Gitlab::Lfs::Router do context 'with required headers' do shared_examples 'responds with a file' do + let(:sendfile) { 'X-Sendfile' } + it 'responds with status 200' do expect(response).to have_http_status(200) end @@ -110,7 +112,6 @@ describe Gitlab::Lfs::Router do context 'with user is authorized' do let(:authorization) { authorize_user } - let(:sendfile) { 'X-Sendfile' } context 'and does not have project access' do let(:update_permissions) do @@ -135,6 +136,10 @@ describe Gitlab::Lfs::Router do context 'when CI is authorized' do let(:authorization) { authorize_ci_project } + let(:update_permissions) do + project.lfs_objects << lfs_object + end + it_behaves_like 'responds with a file' end end From e300dac29302015cbba5b917ad5ea3657de62f0d Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Fri, 15 Jul 2016 15:06:19 +0200 Subject: [PATCH 3/3] Fix LFS specs --- lib/gitlab/lfs/response.rb | 2 ++ lib/gitlab/lfs/router.rb | 2 -- spec/requests/lfs_http_spec.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/gitlab/lfs/response.rb b/lib/gitlab/lfs/response.rb index 811363405a8..a1ee1aa81ff 100644 --- a/lib/gitlab/lfs/response.rb +++ b/lib/gitlab/lfs/response.rb @@ -47,6 +47,8 @@ module Gitlab end def render_storage_upload_store_response(oid, size, tmp_file_name) + return render_forbidden unless tmp_file_name + render_response_to_push do render_lfs_upload_ok(oid, size, tmp_file_name) end diff --git a/lib/gitlab/lfs/router.rb b/lib/gitlab/lfs/router.rb index 69bd5e62305..f2a76a56b8f 100644 --- a/lib/gitlab/lfs/router.rb +++ b/lib/gitlab/lfs/router.rb @@ -74,8 +74,6 @@ module Gitlab lfs.render_storage_upload_authorize_response(oid, size) else tmp_file_name = sanitize_tmp_filename(@request.env['HTTP_X_GITLAB_LFS_TMP']) - return nil unless tmp_file_name - lfs.render_storage_upload_store_response(oid, size, tmp_file_name) end end diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index aaad1e3a6ec..93d2bc160cc 100644 --- a/spec/requests/lfs_http_spec.rb +++ b/spec/requests/lfs_http_spec.rb @@ -557,7 +557,7 @@ describe Gitlab::Lfs::Router do end it 'does not recognize it as a valid lfs command' do - expect(response).to have_http_status(401) + expect(response).to have_http_status(403) end end end