From f3c8374e9bf9b32e46e220df9fb0637636a7474a Mon Sep 17 00:00:00 2001 From: GriffinJ Date: Wed, 13 Feb 2019 12:04:33 -0500 Subject: [PATCH] Ensuring that Dropbox downloads are not placed within a directory outside of the Rails app; Providing a configuration option for the Dropbox provider download directory path; Fixing the CircleCI config. for building the internal test app. using engine_cart --- .circleci/config.yml | 2 +- lib/browse_everything/driver/dropbox.rb | 17 ++++++++++++++- lib/browse_everything/retriever.rb | 9 ++++++-- .../browse_everything_providers.yml.example | 1 + spec/lib/browse_everything/browser_spec.rb | 8 ++++--- .../browse_everything/driver/dropbox_spec.rb | 21 ++++++++++++++++++- 6 files changed, 50 insertions(+), 8 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 2d4eb45d..c89482b3 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -75,7 +75,7 @@ jobs: command: bundle check || bundle install - run: name: Generate test app - command: (git diff --name-only master | grep -qG 'generators\/browse_everything') || bundle exec rake engine_cart:generate + command: bundle exec rake engine_cart:generate - run: name: Ensure test app dependencies are installed command: | diff --git a/lib/browse_everything/driver/dropbox.rb b/lib/browse_everything/driver/dropbox.rb index d68b8bb6..16a48ef6 100644 --- a/lib/browse_everything/driver/dropbox.rb +++ b/lib/browse_everything/driver/dropbox.rb @@ -91,7 +91,7 @@ def downloaded_file_for(path) return @downloaded_files[path] if @downloaded_files.key?(path) # This ensures that the name of the file its extension are preserved for user downloads - temp_file_path = File.join(Dir.mktmpdir, File.basename(path)) + temp_file_path = File.join(download_directory_path, File.basename(path)) temp_file = File.open(temp_file_path, mode: 'w+', encoding: 'ascii-8bit') client.download(path) do |chunk| temp_file.write chunk @@ -160,6 +160,21 @@ def client def redirect_uri(url_options) connector_response_url(**url_options) end + + # Ensures that the "tmp" directory is used if there is no default download + # directory specified in the configuration + # @return [String] + def default_download_directory + Rails.root.join('tmp') + end + + # Retrieves the directory path for downloads used when retrieving the + # resource from Dropbox + # @return [String] + def download_directory_path + dir_path = config[:download_directory] || default_download_directory + File.expand_path(dir_path) + end end end end diff --git a/lib/browse_everything/retriever.rb b/lib/browse_everything/retriever.rb index 8b34e00d..b80f4a6f 100644 --- a/lib/browse_everything/retriever.rb +++ b/lib/browse_everything/retriever.rb @@ -31,8 +31,13 @@ class Retriever attr_accessor :chunk_size class << self - def can_retrieve?(uri) - Typhoeus.get(uri, headers: { Range: 'bytes=0-0' }).success? + # Determines whether or not a remote resource can be retrieved + # @param uri [String] URI for the remote resource (usually a URL) + # @param headers [Hash] any custom headers required to transit the request + def can_retrieve?(uri, headers = {}) + request_headers = headers.merge(Range: 'bytes=0-0') + response = Typhoeus.get(uri, headers: request_headers) + response.success? end end diff --git a/lib/generators/browse_everything/templates/browse_everything_providers.yml.example b/lib/generators/browse_everything/templates/browse_everything_providers.yml.example index 71f89a8b..92866728 100644 --- a/lib/generators/browse_everything/templates/browse_everything_providers.yml.example +++ b/lib/generators/browse_everything/templates/browse_everything_providers.yml.example @@ -5,6 +5,7 @@ # dropbox: # client_id: YOUR_DROPBOX_APP_KEY # client_secret: YOUR_DROPBOX_APP_SECRET +# download_directory: tmp/ # box: # client_id: YOUR_BOX_CLIENT_ID # client_secret: YOUR_BOX_CLIENT_SECRET diff --git a/spec/lib/browse_everything/browser_spec.rb b/spec/lib/browse_everything/browser_spec.rb index 6e891d0f..203472ba 100644 --- a/spec/lib/browse_everything/browser_spec.rb +++ b/spec/lib/browse_everything/browser_spec.rb @@ -4,7 +4,7 @@ describe BrowseEverything::Browser do let(:file_config) do - "file_system:\n home: '/file/config/home'\ndropbox:\n client_id: 'DropboxId'\n client_secret: 'DropboxClientSecret'" + "file_system:\n home: '/file/config/home'\ndropbox:\n client_id: 'DropboxId'\n client_secret: 'DropboxClientSecret'\n download_directory: 'tmp/'" end let(:global_config) do @@ -14,7 +14,8 @@ }, dropbox: { client_id: 'DropboxId', - client_secret: 'DropboxClientSecret' + client_secret: 'DropboxClientSecret', + download_directory: 'tmp/' } } end @@ -26,7 +27,8 @@ }, dropbox: { client_id: 'DropboxId', - client_secret: 'DropboxClientSecret' + client_secret: 'DropboxClientSecret', + download_directory: 'tmp/' }, url_options: url_options } diff --git a/spec/lib/browse_everything/driver/dropbox_spec.rb b/spec/lib/browse_everything/driver/dropbox_spec.rb index d2d6b386..1693666f 100644 --- a/spec/lib/browse_everything/driver/dropbox_spec.rb +++ b/spec/lib/browse_everything/driver/dropbox_spec.rb @@ -8,7 +8,8 @@ let(:provider_yml) do { client_id: 'client-id', - client_secret: 'client-secret' + client_secret: 'client-secret', + download_directory: 'spec/fixtures/' } end let(:oauth_response_body) do @@ -63,6 +64,10 @@ before { driver.connect({ code: 'code' }, {}, connector_response_url_options) } + it 'accesses the download directory' do + expect(driver.config).to include('download_directory' => 'spec/fixtures/') + end + describe '#auth_link' do subject { driver.auth_link(connector_response_url_options) } @@ -133,6 +138,20 @@ expect(downloaded_file.read).not_to be_empty end end + + context 'when the configuration does not specify a download directory' do + let(:provider_yml) do + { + client_id: 'client-id', + client_secret: 'client-secret' + } + end + let(:downloaded_file_path) { link_args.first } + + it 'uses the "tmp" directory as the default path' do + expect(downloaded_file_path).to include '/tmp/Getting Started.pdf' + end + end end end end