Downloading attachments with basic auth

[bobbrodie]

@marlinpierce the new convenience methods you've added for attachments are incredibly helpful. There's one more thing I think we might need to consider, which is that default_headers isn't set when using basic auth.

Describe the solution you'd like
I think it would be great to pass basic authentication to the attachment URL without needing to manually setting the headers.

Describe alternatives you've considered
This is a working example of what I've done as an interim solution:

# Find my issue
issue = client.Issue.find('TP-1')

# Get the first attachment
attachment = issue.attachments.first

# Create an authorization header value by base64 encoding my username and
# password (which in my case is an API token)
auth = Base64.strict_encode64("#{client.options[:username]}:#{client.options[:password]}").chomp

# Download
File.open('attachment.png', 'wb') do |output|
  attachment.download_file({ 'Authorization' => "Basic #{auth}" }) do |file|
    IO.copy_stream(file, output)
  end
end

I'm working on going through the issues and writing out the wiki to prepare for the new version so this is something I should be able to take a look at.

[marlinpierce]

@bobbrodie This PR may have set the default headers.

#415

[bobbrodie]

@bobbrodie This PR may have set the default headers.

#415

Yep @marlinpierce, it does but basic auth isn't using default_headers since username and password are part of the options. I think what we might want to is think about running it through Client so we can do a GET with all the client's configuration included. I'm testing this out but getting an HTTP 303 so I'm adding ?redirect=false per the API. It's almost working, then I'll branch it out and it should work for all authentication methods.

[bobbrodie]

@marlinpierce I have a proposal for this that works, but wanted to run it by you since you authored the download methods.

My goal here is to have the attachment download functionality use the client that's already instantiated. That will ensure that headers will be passed no matter what authentication mechanism a user chooses.

I have a modified version of the download_file method (and corresponding attachment_path like this:

def download_file
  client.get(attachment_path(client, id)).body
end

private
def attachment_path(client, id)
  "#{client.options[:rest_base_path]}/attachment/content/#{id}?redirect=false"
end

This will utilize what the client has implemented to download the attachment, by making a GET such as:

/rest/api/2/attachment/content/10002?redirect=false

I think if we do this then we wouldn't need to pass a block, but a caveat is that I think we'll lose the streaming method -- I wasn't sure if that was something you're actively using or implemented for convenience.

This method will simplify an end-user's implementation to something like this:

# Note
# This is super rudimentary but is a working example

issue = client.Issue.find('TP-1')
attachment = issue.attachments[0]

begin
  File.open("./downloads/#{attachment.filename}", 'wb') do |file|
    file.write(attachment.download_file)
  end
rescue => e
  puts e
end

[marlinpierce]

I developed and tested #415 to use the bearer header. So I think it works with other headers than username and password.

In particular we are using OAuth 2.0 so we set the Access Token in the bearer header.

[marlinpierce]

Oh, I see. Calling URI doesn't use the headers. Gotcha.

[marlinpierce]

We are not using it because it isn't released. I only used it for testing. However, we do use the streaming feature.

The reason for the block is to get the file stream for reading without reading the entire contents. One use case for this is streaming a response. We proxy a download of the Jira attachment. For our HTTP response, we stream the bytes of the file as we have read them, a chunk at a time. This way, the write can start before reading the entire contents.

Already with my code, the calling code which calls download_file does not need to set headers unless it has additional ones not in the default headers. Making the call through client.get is fine though, I don't see any problem.

A connivence method to take an output file name would be ok, but the block version is supposed to support never having to save the contents in either memory or on the file system.

[marlinpierce]

The example given in the documentation for the download_file method shows an example for the use case of streaming the HTTP response.

[marlinpierce]

I think the call to the body method,

client.get(attachment_path(client, id)).body

reads the whole contents into memory and creates a ruby string (or native code string object). The idea was to have one convenience method to read it as a string and a primitive to avoid reading it all into memory or disk. An additional middle ground method which saves to a file but doesn't create a ruby string would be ok, but it should not create the ruby string internally to write the file.

We do have a use case for streaming. That is in use, so yes we need it.

[marlinpierce]

I came from a background of implementing BLOBs for an ODBC and a JDBC driver. The rule was never assume you have enough space to read a BLOB. That was back when a megabyte file was very large.

We are using Jira to replace bugzilla. Our bugzilla bugs do have attachments over a gigabyte. Jira has a hard limit of maybe 2 gigabytes, and our setting is a maximum of 450 megabytes, but it can be good to not read the entire contents. At the very least it allows starting the write of the HTTP response before the read over the network has finished. A 450 MByte file can take a while to load so starting the network write before it finishes is a performance benefit.

[bobbrodie]

This is incredible feedback, thank you so much. I really appreciate the background and will keep thinking this through to see if we can use the client and streaming with all forms of auth and if not, then sort out a clean way to inherit all necessary auth headers.

[marlinpierce]

An example of headers to pass to download_file which are not set in the client is the accept header. You might have an idea of what the content type of the file is and appreciate if the server happens to fail if it cannot confirm the content is acceptable.