diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-03-04 21:07:54 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-03-04 21:07:54 +0000 |
commit | 2fd92f2dc784ade9cb4e1c33dd60cbfad7b86818 (patch) | |
tree | 7779f36689db97a46e0268a4aec1d49f283eb0c8 /spec/uploaders | |
parent | 42ca24aa5bbab7a2d43bc866d9bee9876941cea2 (diff) | |
download | gitlab-ce-2fd92f2dc784ade9cb4e1c33dd60cbfad7b86818.tar.gz |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec/uploaders')
-rw-r--r-- | spec/uploaders/file_mover_spec.rb | 24 | ||||
-rw-r--r-- | spec/uploaders/file_uploader_spec.rb | 44 | ||||
-rw-r--r-- | spec/uploaders/personal_file_uploader_spec.rb | 7 |
3 files changed, 52 insertions, 23 deletions
diff --git a/spec/uploaders/file_mover_spec.rb b/spec/uploaders/file_mover_spec.rb index 9f053f03b0e..474515b537c 100644 --- a/spec/uploaders/file_mover_spec.rb +++ b/spec/uploaders/file_mover_spec.rb @@ -7,7 +7,7 @@ describe FileMover do let(:user) { create(:user) } let(:filename) { 'banana_sample.gif' } - let(:secret) { 'secret55' } + let(:secret) { SecureRandom.hex } let(:temp_file_path) { File.join("uploads/-/system/user/#{user.id}", secret, filename) } let(:temp_description) do @@ -47,8 +47,8 @@ describe FileMover do subject expect(snippet.reload.description) - .to eq("test ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif) "\ - "same ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif) ") + .to eq("test ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/#{secret}/banana_sample.gif) "\ + "same ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/#{secret}/banana_sample.gif) ") end it 'updates existing upload record' do @@ -75,8 +75,8 @@ describe FileMover do subject expect(snippet.reload.description) - .to eq("test ![banana_sample](/uploads/-/system/user/#{user.id}/secret55/banana_sample.gif) "\ - "same ![banana_sample](/uploads/-/system/user/#{user.id}/secret55/banana_sample.gif) ") + .to eq("test ![banana_sample](/uploads/-/system/user/#{user.id}/#{secret}/banana_sample.gif) "\ + "same ![banana_sample](/uploads/-/system/user/#{user.id}/#{secret}/banana_sample.gif) ") end it 'does not change the upload record' do @@ -101,8 +101,8 @@ describe FileMover do subject expect(snippet.reload.description) - .to eq("test ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif) "\ - "same ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/secret55/banana_sample.gif) ") + .to eq("test ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/#{secret}/banana_sample.gif) "\ + "same ![banana_sample](/uploads/-/system/personal_snippet/#{snippet.id}/#{secret}/banana_sample.gif) ") end it 'creates new target upload record an delete the old upload' do @@ -121,8 +121,8 @@ describe FileMover do subject expect(snippet.reload.description) - .to eq("test ![banana_sample](/uploads/-/system/user/#{user.id}/secret55/banana_sample.gif) "\ - "same ![banana_sample](/uploads/-/system/user/#{user.id}/secret55/banana_sample.gif) ") + .to eq("test ![banana_sample](/uploads/-/system/user/#{user.id}/#{secret}/banana_sample.gif) "\ + "same ![banana_sample](/uploads/-/system/user/#{user.id}/#{secret}/banana_sample.gif) ") end it 'does not change the upload record' do @@ -138,12 +138,8 @@ describe FileMover do let(:temp_file_path) { File.join("uploads/-/system/user/#{user.id}", '..', 'another_subdir_of_temp') } it 'does not trigger move if path is outside designated directory' do - stub_file_mover("uploads/-/system/user/#{user.id}/another_subdir_of_temp") expect(FileUtils).not_to receive(:move) - - subject - - expect(snippet.reload.description).to eq(temp_description) + expect { subject }.to raise_error(FileUploader::InvalidSecret) end end diff --git a/spec/uploaders/file_uploader_spec.rb b/spec/uploaders/file_uploader_spec.rb index beb7aa3cf2c..efdbea85d4a 100644 --- a/spec/uploaders/file_uploader_spec.rb +++ b/spec/uploaders/file_uploader_spec.rb @@ -6,7 +6,8 @@ describe FileUploader do let(:group) { create(:group, name: 'awesome') } let(:project) { create(:project, :legacy_storage, namespace: group, name: 'project') } let(:uploader) { described_class.new(project, :avatar) } - let(:upload) { double(model: project, path: 'secret/foo.jpg') } + let(:upload) { double(model: project, path: "#{secret}/foo.jpg") } + let(:secret) { "55dc16aa0edd05693fd98b5051e83321" } # this would be nicer as SecureRandom.hex, but the shared_examples breaks subject { uploader } @@ -14,7 +15,7 @@ describe FileUploader do include_examples 'builds correct paths', store_dir: %r{awesome/project/\h+}, upload_path: %r{\h+/<filename>}, - absolute_path: %r{#{described_class.root}/awesome/project/secret/foo.jpg} + absolute_path: %r{#{described_class.root}/awesome/project/55dc16aa0edd05693fd98b5051e83321/foo.jpg} end context 'legacy storage' do @@ -51,11 +52,11 @@ describe FileUploader do end describe 'initialize' do - let(:uploader) { described_class.new(double, secret: 'secret') } + let(:uploader) { described_class.new(double, secret: secret) } it 'accepts a secret parameter' do expect(described_class).not_to receive(:generate_secret) - expect(uploader.secret).to eq('secret') + expect(uploader.secret).to eq(secret) end end @@ -154,8 +155,39 @@ describe FileUploader do describe '#secret' do it 'generates a secret if none is provided' do - expect(described_class).to receive(:generate_secret).and_return('secret') - expect(uploader.secret).to eq('secret') + expect(described_class).to receive(:generate_secret).and_return(secret) + expect(uploader.secret).to eq(secret) + expect(uploader.secret.size).to eq(32) + end + + context "validation" do + before do + uploader.instance_variable_set(:@secret, secret) + end + + context "32-byte hexadecimal" do + let(:secret) { SecureRandom.hex } + + it "returns the secret" do + expect(uploader.secret).to eq(secret) + end + end + + context "10-byte hexadecimal" do + let(:secret) { SecureRandom.hex(10) } + + it "returns the secret" do + expect(uploader.secret).to eq(secret) + end + end + + context "invalid secret supplied" do + let(:secret) { "%2E%2E%2F%2E%2E%2F%2E%2E%2F%2E%2E%2F%2E%2E%2F%2E%2E%2F%2E%2E%2Fgrafana%2Fconf%2F" } + + it "raises an exception" do + expect { uploader.secret }.to raise_error(described_class::InvalidSecret) + end + end end end diff --git a/spec/uploaders/personal_file_uploader_spec.rb b/spec/uploaders/personal_file_uploader_spec.rb index ec652af222d..c211ec3607c 100644 --- a/spec/uploaders/personal_file_uploader_spec.rb +++ b/spec/uploaders/personal_file_uploader_spec.rb @@ -6,12 +6,13 @@ describe PersonalFileUploader do let(:model) { create(:personal_snippet) } let(:uploader) { described_class.new(model) } let(:upload) { create(:upload, :personal_snippet_upload) } + let(:secret) { SecureRandom.hex } subject { uploader } shared_examples '#base_dir' do before do - subject.instance_variable_set(:@secret, 'secret') + subject.instance_variable_set(:@secret, secret) end it 'is prefixed with uploads/-/system' do @@ -23,12 +24,12 @@ describe PersonalFileUploader do shared_examples '#to_h' do before do - subject.instance_variable_set(:@secret, 'secret') + subject.instance_variable_set(:@secret, secret) end it 'is correct' do allow(uploader).to receive(:file).and_return(double(extension: 'txt', filename: 'file_name')) - expected_url = "/uploads/-/system/personal_snippet/#{model.id}/secret/file_name" + expected_url = "/uploads/-/system/personal_snippet/#{model.id}/#{secret}/file_name" expect(uploader.to_h).to eq( alt: 'file_name', |