diff options
author | Ahmad Sherif <me@ahmadsherif.com> | 2019-07-22 17:56:40 +0300 |
---|---|---|
committer | Ahmad Sherif <me@ahmadsherif.com> | 2019-09-10 14:43:11 +0300 |
commit | 3c2b4a1cede956d5160ccf08d0a561bf31248161 (patch) | |
tree | 9462f59d477ffe7ac1eee0fe56cf9f343b568d1f /spec | |
parent | f7e7ee713aa21874bf6810d01976c2b5342c0995 (diff) |
Enable serving static objects from an external storage
It consists of two parts:
1. Redirecting users to the configured external storage
1. Allowing the external storage to request the static object(s)
on behalf of the user by means of specific tokens
Part of https://gitlab.com/gitlab-com/gl-infra/infrastructure/issues/6829
Diffstat (limited to 'spec')
11 files changed, 343 insertions, 1 deletions
diff --git a/spec/controllers/concerns/static_object_external_storage_spec.rb b/spec/controllers/concerns/static_object_external_storage_spec.rb new file mode 100644 index 00000000000..3a0219ddaa1 --- /dev/null +++ b/spec/controllers/concerns/static_object_external_storage_spec.rb @@ -0,0 +1,96 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe StaticObjectExternalStorage do + controller(Projects::ApplicationController) do + include StaticObjectExternalStorage # rubocop:disable RSpec/DescribedClass + + before_action :redirect_to_external_storage, if: :static_objects_external_storage_enabled? + + def show + head :ok + end + end + + let(:project) { create(:project, :public) } + let(:user) { create(:user, static_object_token: 'hunter1') } + + before do + project.add_developer(user) + sign_in(user) + end + + context 'when external storage is not configured' do + it 'calls the action normally' do + expect(Gitlab::CurrentSettings.static_objects_external_storage_url).to be_blank + + do_request + + expect(response).to have_gitlab_http_status(200) + end + end + + context 'when external storage is configured' do + before do + allow_any_instance_of(ApplicationSetting).to receive(:static_objects_external_storage_url).and_return('https://cdn.gitlab.com') + allow_any_instance_of(ApplicationSetting).to receive(:static_objects_external_storage_auth_token).and_return('letmein') + + routes.draw { get '/:namespace_id/:id' => 'projects/application#show' } + end + + context 'when external storage token is empty' do + let(:base_redirect_url) { "https://cdn.gitlab.com/#{project.namespace.to_param}/#{project.to_param}" } + + context 'when project is public' do + it 'redirects to external storage URL without adding a token parameter' do + do_request + + expect(response).to redirect_to(base_redirect_url) + end + end + + context 'when project is not public' do + let(:project) { create(:project, :private) } + + it 'redirects to external storage URL a token parameter added' do + do_request + + expect(response).to redirect_to("#{base_redirect_url}?token=#{user.static_object_token}") + end + + context 'when path includes extra parameters' do + it 'includes the parameters in the redirect URL' do + do_request(foo: 'bar') + + expect(response.location).to eq("#{base_redirect_url}?foo=bar&token=#{user.static_object_token}") + end + end + end + end + + context 'when external storage token is present' do + context 'when token is correct' do + it 'calls the action normally' do + request.headers['X-Gitlab-External-Storage-Token'] = 'letmein' + do_request + + expect(response).to have_gitlab_http_status(200) + end + end + + context 'when token is incorrect' do + it 'return 403' do + request.headers['X-Gitlab-External-Storage-Token'] = 'donotletmein' + do_request + + expect(response).to have_gitlab_http_status(403) + end + end + end + end + + def do_request(extra_params = {}) + get :show, params: { namespace_id: project.namespace, id: project }.merge(extra_params) + end +end diff --git a/spec/controllers/projects/repositories_controller_spec.rb b/spec/controllers/projects/repositories_controller_spec.rb index fcab4d73dca..084644484c5 100644 --- a/spec/controllers/projects/repositories_controller_spec.rb +++ b/spec/controllers/projects/repositories_controller_spec.rb @@ -125,5 +125,59 @@ describe Projects::RepositoriesController do end end end + + context 'as a sessionless user' do + let(:user) { create(:user) } + + before do + project.add_developer(user) + end + + context 'when no token is provided' do + it 'redirects to sign in page' do + get :archive, params: { namespace_id: project.namespace, project_id: project, id: 'master' }, format: 'zip' + + expect(response).to have_gitlab_http_status(302) + end + end + + context 'when a token param is present' do + context 'when token is correct' do + it 'calls the action normally' do + get :archive, params: { namespace_id: project.namespace, project_id: project, id: 'master', token: user.static_object_token }, format: 'zip' + + expect(response).to have_gitlab_http_status(200) + end + end + + context 'when token is incorrect' do + it 'redirects to sign in page' do + get :archive, params: { namespace_id: project.namespace, project_id: project, id: 'master', token: 'foobar' }, format: 'zip' + + expect(response).to have_gitlab_http_status(302) + end + end + end + + context 'when a token header is present' do + context 'when token is correct' do + it 'calls the action normally' do + request.headers['X-Gitlab-Static-Object-Token'] = user.static_object_token + get :archive, params: { namespace_id: project.namespace, project_id: project, id: 'master' }, format: 'zip' + + expect(response).to have_gitlab_http_status(200) + end + end + + context 'when token is incorrect' do + it 'redirects to sign in page' do + request.headers['X-Gitlab-Static-Object-Token'] = 'foobar' + get :archive, params: { namespace_id: project.namespace, project_id: project, id: 'master' }, format: 'zip' + + expect(response).to have_gitlab_http_status(302) + end + end + end + end end end diff --git a/spec/features/projects/branches/download_buttons_spec.rb b/spec/features/projects/branches/download_buttons_spec.rb index 401425187b0..e0b0e22823e 100644 --- a/spec/features/projects/branches/download_buttons_spec.rb +++ b/spec/features/projects/branches/download_buttons_spec.rb @@ -29,6 +29,11 @@ describe 'Download buttons in branches page' do end describe 'when checking branches' do + it_behaves_like 'archive download buttons' do + let(:ref) { 'binary-encoding' } + let(:path_to_visit) { project_branches_filtered_path(project, state: 'all', search: ref) } + end + context 'with artifacts' do before do visit project_branches_filtered_path(project, state: 'all', search: 'binary-encoding') diff --git a/spec/features/projects/files/download_buttons_spec.rb b/spec/features/projects/files/download_buttons_spec.rb index a4889f8d4c4..871f5212ddd 100644 --- a/spec/features/projects/files/download_buttons_spec.rb +++ b/spec/features/projects/files/download_buttons_spec.rb @@ -24,11 +24,17 @@ describe 'Projects > Files > Download buttons in files tree' do before do sign_in(user) project.add_developer(user) + end - visit project_tree_path(project, project.default_branch) + it_behaves_like 'archive download buttons' do + let(:path_to_visit) { project_tree_path(project, project.default_branch) } end context 'with artifacts' do + before do + visit project_tree_path(project, project.default_branch) + end + it 'shows download artifacts button' do href = latest_succeeded_project_artifacts_path(project, "#{project.default_branch}/download", job: 'build') diff --git a/spec/features/projects/show/download_buttons_spec.rb b/spec/features/projects/show/download_buttons_spec.rb index 5e7453bcdb7..0d609069426 100644 --- a/spec/features/projects/show/download_buttons_spec.rb +++ b/spec/features/projects/show/download_buttons_spec.rb @@ -29,6 +29,8 @@ describe 'Projects > Show > Download buttons' do end describe 'when checking project main page' do + it_behaves_like 'archive download buttons' + context 'with artifacts' do before do visit project_path(project) diff --git a/spec/features/projects/tags/download_buttons_spec.rb b/spec/features/projects/tags/download_buttons_spec.rb index 76b2704ae49..64141cf5dc9 100644 --- a/spec/features/projects/tags/download_buttons_spec.rb +++ b/spec/features/projects/tags/download_buttons_spec.rb @@ -30,6 +30,11 @@ describe 'Download buttons in tags page' do end describe 'when checking tags' do + it_behaves_like 'archive download buttons' do + let(:path_to_visit) { project_tags_path(project) } + let(:ref) { tag } + end + context 'with artifacts' do before do visit project_tags_path(project) diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index b81249a1e29..4a3ff7e0095 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -195,4 +195,41 @@ describe ApplicationHelper do end end end + + describe '#external_storage_url_or_path' do + let(:project) { create(:project) } + + context 'when external storage is disabled' do + it 'returns the passed path' do + expect(helper.external_storage_url_or_path('/foo/bar', project)).to eq('/foo/bar') + end + end + + context 'when external storage is enabled' do + let(:user) { create(:user, static_object_token: 'hunter1') } + + before do + allow_any_instance_of(ApplicationSetting).to receive(:static_objects_external_storage_url).and_return('https://cdn.gitlab.com') + allow(helper).to receive(:current_user).and_return(user) + end + + it 'returns the external storage URL prepended to the path' do + expect(helper.external_storage_url_or_path('/foo/bar', project)).to eq("https://cdn.gitlab.com/foo/bar?token=#{user.static_object_token}") + end + + it 'preserves the path query parameters' do + url = helper.external_storage_url_or_path('/foo/bar?unicode=1', project) + + expect(url).to eq("https://cdn.gitlab.com/foo/bar?token=#{user.static_object_token}&unicode=1") + end + + context 'when project is public' do + let(:project) { create(:project, :public) } + + it 'returns does not append a token parameter' do + expect(helper.external_storage_url_or_path('/foo/bar', project)).to eq('https://cdn.gitlab.com/foo/bar') + end + end + end + end end diff --git a/spec/lib/gitlab/auth/user_auth_finders_spec.rb b/spec/lib/gitlab/auth/user_auth_finders_spec.rb index 41265da97a4..dd8070c1240 100644 --- a/spec/lib/gitlab/auth/user_auth_finders_spec.rb +++ b/spec/lib/gitlab/auth/user_auth_finders_spec.rb @@ -115,6 +115,60 @@ describe Gitlab::Auth::UserAuthFinders do end end + describe '#find_user_from_static_object_token' do + context 'when request format is archive' do + before do + env['SCRIPT_NAME'] = 'project/-/archive/master.zip' + end + + context 'when token header param is present' do + context 'when token is correct' do + it 'returns the user' do + request.headers['X-Gitlab-Static-Object-Token'] = user.static_object_token + + expect(find_user_from_static_object_token(:archive)).to eq(user) + end + end + + context 'when token is incorrect' do + it 'returns the user' do + request.headers['X-Gitlab-Static-Object-Token'] = 'foobar' + + expect { find_user_from_static_object_token(:archive) }.to raise_error(Gitlab::Auth::UnauthorizedError) + end + end + end + + context 'when token query param is present' do + context 'when token is correct' do + it 'returns the user' do + set_param(:token, user.static_object_token) + + expect(find_user_from_static_object_token(:archive)).to eq(user) + end + end + + context 'when token is incorrect' do + it 'returns the user' do + set_param(:token, 'foobar') + + expect { find_user_from_static_object_token(:archive) }.to raise_error(Gitlab::Auth::UnauthorizedError) + end + end + end + end + + context 'when request format is not archive' do + before do + env['script_name'] = 'url' + end + + it 'returns nil' do + expect(find_user_from_static_object_token(:foo)).to be_nil + end + end + end + describe '#find_user_from_access_token' do let(:personal_access_token) { create(:personal_access_token, user: user) } diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 4f7a6d102b8..d12f9b9100a 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -48,6 +48,10 @@ describe ApplicationSetting do it { is_expected.not_to allow_value(nil).for(:outbound_local_requests_whitelist) } it { is_expected.to allow_value([]).for(:outbound_local_requests_whitelist) } + it { is_expected.to allow_value(nil).for(:static_objects_external_storage_url) } + it { is_expected.to allow_value(http).for(:static_objects_external_storage_url) } + it { is_expected.to allow_value(https).for(:static_objects_external_storage_url) } + context "when user accepted let's encrypt terms of service" do before do setting.update(lets_encrypt_terms_of_service_accepted: true) @@ -420,6 +424,16 @@ describe ApplicationSetting do end end end + + context 'static objects external storage' do + context 'when URL is set' do + before do + subject.static_objects_external_storage_url = http + end + + it { is_expected.not_to allow_value(nil).for(:static_objects_external_storage_auth_token) } + end + end end context 'restrict creating duplicates' do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index b8c323904b8..741563292e9 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -945,6 +945,16 @@ describe User do end end + describe 'static object token' do + it 'ensures a static object token on read' do + user = create(:user, static_object_token: nil) + static_object_token = user.static_object_token + + expect(static_object_token).not_to be_blank + expect(user.reload.static_object_token).to eq static_object_token + end + end + describe '#recently_sent_password_reset?' do it 'is false when reset_password_sent_at is nil' do user = build_stubbed(:user, reset_password_sent_at: nil) diff --git a/spec/support/shared_examples/features/archive_download_buttons_shared_examples.rb b/spec/support/shared_examples/features/archive_download_buttons_shared_examples.rb new file mode 100644 index 00000000000..920fcbde483 --- /dev/null +++ b/spec/support/shared_examples/features/archive_download_buttons_shared_examples.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +shared_examples 'archive download buttons' do + let(:formats) { %w(zip tar.gz tar.bz2 tar) } + let(:path_to_visit) { project_path(project) } + let(:ref) { project.default_branch } + + context 'when static objects external storage is enabled' do + before do + allow_any_instance_of(ApplicationSetting).to receive(:static_objects_external_storage_url).and_return('https://cdn.gitlab.com') + visit path_to_visit + end + + context 'private project' do + it 'shows archive download buttons with external storage URL prepended and user token appended to their href' do + formats.each do |format| + path = archive_path(project, ref, format) + uri = URI('https://cdn.gitlab.com') + uri.path = path + uri.query = "token=#{user.static_object_token}" + + expect(page).to have_link format, href: uri.to_s + end + end + end + + context 'public project' do + let(:project) { create(:project, :repository, :public) } + + it 'shows archive download buttons with external storage URL prepended to their href' do + formats.each do |format| + path = archive_path(project, ref, format) + uri = URI('https://cdn.gitlab.com') + uri.path = path + + expect(page).to have_link format, href: uri.to_s + end + end + end + end + + context 'when static objects external storage is disabled' do + before do + visit path_to_visit + end + + it 'shows default archive download buttons' do + formats.each do |format| + path = archive_path(project, ref, format) + + expect(page).to have_link format, href: path + end + end + end + + def archive_path(project, ref, format) + project_archive_path(project, id: "#{ref}/#{project.path}-#{ref}", path: nil, format: format) + end +end |