Skip to content

Commit

Permalink
Merge pull request #265 from jrgriffiniii/dropbox-provider-download-path
Browse files Browse the repository at this point in the history
Ensures that Dropbox downloads are not placed within a directory outside of the Rails app
  • Loading branch information
jrgriffiniii committed Feb 15, 2019
2 parents f3ed0b7 + 7b7ec1c commit ee34db1
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 8 deletions.
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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: |
Expand Down
17 changes: 16 additions & 1 deletion lib/browse_everything/driver/dropbox.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
9 changes: 7 additions & 2 deletions lib/browse_everything/retriever.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 5 additions & 3 deletions spec/lib/browse_everything/browser_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -14,7 +14,8 @@
},
dropbox: {
client_id: 'DropboxId',
client_secret: 'DropboxClientSecret'
client_secret: 'DropboxClientSecret',
download_directory: 'tmp/'
}
}
end
Expand All @@ -26,7 +27,8 @@
},
dropbox: {
client_id: 'DropboxId',
client_secret: 'DropboxClientSecret'
client_secret: 'DropboxClientSecret',
download_directory: 'tmp/'
},
url_options: url_options
}
Expand Down
21 changes: 20 additions & 1 deletion spec/lib/browse_everything/driver/dropbox_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) }

Expand Down Expand Up @@ -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

0 comments on commit ee34db1

Please sign in to comment.