7884: Detect when a "download log" response has a JSON-encoded redirect

to keep-web, and convert it to POST to avoid a second redirect that
would be forbidden by SOP.

diff --git a/apps/workbench/app/views/jobs/_show_log.html.erb b/apps/workbench/app/views/jobs/_show_log.html.erb
index 18021349e2d5e052fe1870bdb35d834f39c6e897..7d67b74210812895347d21d6c08f4ceb15c50db0 100644 (file)
--- a/apps/workbench/app/views/jobs/_show_log.html.erb
+++ b/apps/workbench/app/views/jobs/_show_log.html.erb
@@ -62,38 +62,62 @@ var makeFilter = function() {
 <% if @object.log and !@object.log.empty? %>
   <% logcollection = Collection.find @object.log %>
   <% if logcollection %>
-    log_size = <%= logcollection.files[0][2] %>
-    log_maxbytes = <%= Rails.configuration.log_viewer_max_bytes %>;
-    logcollection_url = '<%=j url_for logcollection %>/<%=j logcollection.files[0][1] %>';
+    var log_size = <%= logcollection.files[0][2] %>
+    var log_maxbytes = <%= Rails.configuration.log_viewer_max_bytes %>;
+    var logcollection_url = '<%=j url_for logcollection %>/<%=j logcollection.files[0][1] %>';
     $("#log-viewer-download-url").attr('href', logcollection_url);
     $("#log-viewer-download-pane").show();
+    var headers = {};
     if (log_size > log_maxbytes) {
-      range_header = { 'Range': 'bytes=0-' + log_maxbytes };
-    } else {
-      range_header = null;
+      headers['Range'] = 'bytes=0-' + log_maxbytes;
     }
-    $.ajax(logcollection_url, { headers: range_header }).
-        done(function(data, status, jqxhr) {
-            logViewer.filter();
-            addToLogViewer(logViewer, data.split("\n"), taskState);
-            logViewer.filter(makeFilter());
-            content_range_hdr = jqxhr.getResponseHeader('Content-Range');
-            var v = content_range_hdr && content_range_hdr.match(/bytes \d+-(\d+)\/(.+)/);
-            short_log = v && (v[2] == '*' || parseInt(v[1]) + 1 < v[2]);
-            if (jqxhr.status == 206 && short_log) {
-              $("#log-viewer-overview").html(
-                '<p>Showing only ' + data.length + ' bytes of this log.' +
-                ' Timing information is unavailable since' +
-                ' the full log was not retrieved.</p>'
-              );
-            } else {
-              generateJobOverview("#log-viewer-overview", logViewer, taskState);
+    var ajax_opts = { dataType: 'text', headers: headers };
+    load_log();
+
+    function load_log() {
+        $.ajax(logcollection_url, ajax_opts).done(done).fail(fail);
+    }
+    function done(data, status, jqxhr) {
+        if (jqxhr.getResponseHeader('Content-Type').indexOf('application/json') === 0) {
+            // The browser won't allow a redirect-with-cookie response
+            // because keep-web isn't same-origin with us. Instead, we
+            // assure keep-web it's OK to respond with the content
+            // immediately by setting the token in the request body
+            // instead and adding disposition=attachment.
+            logcollection_url = JSON.parse(data).href;
+            var queryAt = logcollection_url.indexOf('?api_token=');
+            if (queryAt >= 0) {
+                ajax_opts.method = 'POST';
+                ajax_opts.data = {
+                    api_token: logcollection_url.slice(queryAt+11),
+                    disposition: 'attachment',
+                };
+                logcollection_url = logcollection_url.slice(0, queryAt);
             }
-            $("#log-viewer .spinner").detach();
-        }).
-        fail(function(jqxhr, status, error) {
-            $("#log-viewer .spinner").detach();
-        });
+            return load_log();
+        }
+        logViewer.filter();
+        addToLogViewer(logViewer, data.split("\n"), taskState);
+        logViewer.filter(makeFilter());
+        content_range_hdr = jqxhr.getResponseHeader('Content-Range');
+        var v = content_range_hdr && content_range_hdr.match(/bytes \d+-(\d+)\/(.+)/);
+        short_log = v && (v[2] == '*' || parseInt(v[1]) + 1 < v[2]);
+        if (jqxhr.status == 206 && short_log) {
+            $("#log-viewer-overview").html(
+                '<p>Showing only ' + data.length + ' bytes of this log.' +
+                    ' Timing information is unavailable since' +
+                    ' the full log was not retrieved.</p>'
+            );
+        } else {
+            generateJobOverview("#log-viewer-overview", logViewer, taskState);
+        }
+        $("#log-viewer .spinner").detach();
+    }
+    function fail(jqxhr, status, error) {
+        // TODO: tell the user about the error
+        console.log('load_log failed: status='+status+' error='+error);
+        $("#log-viewer .spinner").detach();
+    }
   <% end %>
 <% else %>
   <%# Live log loading not implemented yet. %>

diff --git a/apps/workbench/test/integration/download_test.rb b/apps/workbench/test/integration/download_test.rb
index ed91ae08695ee96aaabde519e82781ffe4528ea1..8a16fb8a66b547ae704cc2791f06f330a5268bc9 100644 (file)
--- a/apps/workbench/test/integration/download_test.rb
+++ b/apps/workbench/test/integration/download_test.rb
@@ -2,16 +2,10 @@ require 'integration_helper'
 require 'helpers/download_helper'
 
 class DownloadTest < ActionDispatch::IntegrationTest
-  def getport service
-    File.read(File.expand_path("../../../../../tmp/#{service}.port", __FILE__))
-  end
+  include KeepWebConfig
 
   setup do
-    @kwport = getport 'keep-web-ssl'
-    @kwdport = getport 'keep-web-dl-ssl'
-    Rails.configuration.keep_web_url = "https://localhost:#{@kwport}/c=%{uuid_or_pdh}"
-    Rails.configuration.keep_web_download_url = "https://localhost:#{@kwdport}/c=%{uuid_or_pdh}"
-    CollectionsController.any_instance.expects(:file_enumerator).never
+    use_keep_web_config
 
     # Make sure Capybara can download files.
     need_selenium 'for downloading', :selenium_with_download

diff --git a/apps/workbench/test/integration/jobs_test.rb b/apps/workbench/test/integration/jobs_test.rb
index b8d38903797d56eae4b01169efce1c4b169f232b..0c407b3827cb51d4ba25765d1f84d55a24f0b77d 100644 (file)
--- a/apps/workbench/test/integration/jobs_test.rb
+++ b/apps/workbench/test/integration/jobs_test.rb
@@ -4,6 +4,8 @@ require 'tmpdir'
 require 'integration_helper'
 
 class JobsTest < ActionDispatch::IntegrationTest
+  include KeepWebConfig
+
   setup do
       need_javascript
   end
@@ -66,6 +68,26 @@ class JobsTest < ActionDispatch::IntegrationTest
     assert page.has_text? 'Showing only 100 bytes of this log'
   end
 
+  test 'view log via keep-web redirect' do
+    use_keep_web_config
+
+    token = api_fixture('api_client_authorizations')['active']['api_token']
+    logdata = fakepipe_with_log_data.read
+    logblock = `echo -n #{logdata.shellescape} | ARVADOS_API_TOKEN=#{token.shellescape} arv-put --no-progress --raw -`.strip
+    assert $?.success?, $?
+
+    job = nil
+    use_token 'active' do
+      job = Job.find api_fixture('jobs')['running']['uuid']
+      mtxt = ". #{logblock} 0:#{logdata.length}:#{job.uuid}.log.txt\n"
+      logcollection = Collection.create(manifest_text: mtxt)
+      job.update_attributes log: logcollection.portable_data_hash
+    end
+    visit page_with_token 'active', '/jobs/'+job.uuid
+    find('a[href="#Log"]').click
+    assert_text 'log message 1'
+  end
+
   [
     ['foobar', false, false],
     ['job_with_latest_version', true, false],

diff --git a/apps/workbench/test/integration_helper.rb b/apps/workbench/test/integration_helper.rb
index 48d2cb5f6d3123513e345a24c5489d2893fb3f2e..785912d3242e303825fce26975fa38b314ac1e88 100644 (file)
--- a/apps/workbench/test/integration_helper.rb
+++ b/apps/workbench/test/integration_helper.rb
@@ -126,6 +126,20 @@ module HeadlessHelper
   end
 end
 
+module KeepWebConfig
+  def getport service
+    File.read(File.expand_path("../../../../tmp/#{service}.port", __FILE__))
+  end
+
+  def use_keep_web_config
+    @kwport = getport 'keep-web-ssl'
+    @kwdport = getport 'keep-web-dl-ssl'
+    Rails.configuration.keep_web_url = "https://localhost:#{@kwport}/c=%{uuid_or_pdh}"
+    Rails.configuration.keep_web_download_url = "https://localhost:#{@kwdport}/c=%{uuid_or_pdh}"
+    CollectionsController.any_instance.expects(:file_enumerator).never
+  end
+end
+
 class ActionDispatch::IntegrationTest
   # Make the Capybara DSL available in all integration tests
   include Capybara::DSL