[ARVADOS] updated: 2b699dec710d1f0719ec0471bc711a467ac34a95
git at public.curoverse.com
git at public.curoverse.com
Fri Dec 18 15:42:14 EST 2015
Summary of changes:
apps/workbench/app/views/jobs/_show_log.html.erb | 78 ++++++++++++++++--------
apps/workbench/test/integration/download_test.rb | 10 +--
apps/workbench/test/integration/jobs_test.rb | 22 +++++++
apps/workbench/test/integration_helper.rb | 14 +++++
services/keep-web/handler.go | 30 ++++++++-
services/keep-web/handler_test.go | 56 ++++++++++++++++-
6 files changed, 170 insertions(+), 40 deletions(-)
via 2b699dec710d1f0719ec0471bc711a467ac34a95 (commit)
via ed3efbfa0fd595fbf88968f9b42e7722993f44c2 (commit)
via 65f8a7847469f976f3113f970dcc11f81cc040ef (commit)
via 1f1991b4824272e9b5a4b37eee6931e6519f8bdf (commit)
from ace5807988dd1db2e8bd63a788fb0f0d9da152d7 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit 2b699dec710d1f0719ec0471bc711a467ac34a95
Merge: ace5807 ed3efbf
Author: Tom Clegg <tom at curoverse.com>
Date: Fri Dec 18 15:40:40 2015 -0500
Merge branch '7884-ajax-log-redirect' closes #7884
commit ed3efbfa0fd595fbf88968f9b42e7722993f44c2
Author: Tom Clegg <tom at curoverse.com>
Date: Fri Dec 18 15:40:11 2015 -0500
7884: Clarify "credentials" comment.
diff --git a/services/keep-web/handler.go b/services/keep-web/handler.go
index c947a6e..e1b2362 100644
--- a/services/keep-web/handler.go
+++ b/services/keep-web/handler.go
@@ -101,8 +101,11 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
}
if r.Header.Get("Origin") != "" {
- // Allow simple cross-origin requests, without
- // credentials.
+ // Allow simple cross-origin requests without user
+ // credentials ("user credentials" as defined by CORS,
+ // i.e., cookies, HTTP authentication, and client-side
+ // SSL certificates. See
+ // http://www.w3.org/TR/cors/#user-credentials).
w.Header().Set("Access-Control-Allow-Origin", "*")
}
commit 65f8a7847469f976f3113f970dcc11f81cc040ef
Author: Tom Clegg <tom at curoverse.com>
Date: Thu Dec 17 01:07:32 2015 -0500
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 1802134..7d67b74 100644
--- 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 ed91ae0..8a16fb8 100644
--- 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 b8d3890..0c407b3 100644
--- 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 48d2cb5..785912d 100644
--- 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
commit 1f1991b4824272e9b5a4b37eee6931e6519f8bdf
Author: Tom Clegg <tom at curoverse.com>
Date: Thu Dec 17 01:03:37 2015 -0500
7884: Serve simple cross-origin AJAX POST requests without redirecting.
diff --git a/services/keep-web/handler.go b/services/keep-web/handler.go
index 847329f..c947a6e 100644
--- a/services/keep-web/handler.go
+++ b/services/keep-web/handler.go
@@ -100,6 +100,12 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
return
}
+ if r.Header.Get("Origin") != "" {
+ // Allow simple cross-origin requests, without
+ // credentials.
+ w.Header().Set("Access-Control-Allow-Origin", "*")
+ }
+
arv := clientPool.Get()
if arv == nil {
statusCode, statusText = http.StatusInternalServerError, "Pool failed: "+clientPool.Err().Error()
@@ -149,7 +155,24 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
statusCode = http.StatusNotFound
return
}
- if t := r.FormValue("api_token"); t != "" {
+
+ formToken := r.FormValue("api_token")
+ if formToken != "" && r.Header.Get("Origin") != "" && attachment && r.URL.Query().Get("api_token") == "" {
+ // The client provided an explicit token in the POST
+ // body. The Origin header indicates this *might* be
+ // an AJAX request, in which case redirect-with-cookie
+ // won't work: we should just serve the content in the
+ // POST response. This is safe because:
+ //
+ // * We're supplying an attachment, not inline
+ // content, so we don't need to convert the POST to
+ // a GET and avoid the "really resubmit form?"
+ // problem.
+ //
+ // * The token isn't embedded in the URL, so we don't
+ // need to worry about bookmarks and copy/paste.
+ tokens = append(tokens, formToken)
+ } else if formToken != "" {
// The client provided an explicit token in the query
// string, or a form in POST body. We must put the
// token in an HttpOnly cookie, and redirect to the
@@ -179,7 +202,7 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
http.SetCookie(w, &http.Cookie{
Name: "arvados_api_token",
- Value: auth.EncodeTokenCookie([]byte(t)),
+ Value: auth.EncodeTokenCookie([]byte(formToken)),
Path: "/",
HttpOnly: true,
})
diff --git a/services/keep-web/handler_test.go b/services/keep-web/handler_test.go
index 94a1cf7..d04c5c2 100644
--- a/services/keep-web/handler_test.go
+++ b/services/keep-web/handler_test.go
@@ -96,6 +96,21 @@ func authzViaPOST(r *http.Request, tok string) int {
return http.StatusUnauthorized
}
+func (s *IntegrationSuite) TestVhostViaXHRPOST(c *check.C) {
+ doVhostRequests(c, authzViaPOST)
+}
+func authzViaXHRPOST(r *http.Request, tok string) int {
+ r.Method = "POST"
+ r.Header.Add("Content-Type", "application/x-www-form-urlencoded")
+ r.Header.Add("Origin", "https://origin.example")
+ r.Body = ioutil.NopCloser(strings.NewReader(
+ url.Values{
+ "api_token": {tok},
+ "disposition": {"attachment"},
+ }.Encode()))
+ return http.StatusUnauthorized
+}
+
// Try some combinations of {url, token} using the given authorization
// mechanism, and verify the result is correct.
func doVhostRequests(c *check.C, authz authorizer) {
@@ -129,11 +144,18 @@ func doVhostRequestsWithHostPath(c *check.C, authz authorizer, hostPath string)
Header: http.Header{},
}
failCode := authz(req, tok)
- resp := doReq(req)
+ req, resp := doReq(req)
code, body := resp.Code, resp.Body.String()
+
+ // If the initial request had a (non-empty) token
+ // showing in the query string, we should have been
+ // redirected in order to hide it in a cookie.
+ c.Check(req.URL.String(), check.Not(check.Matches), `.*api_token=.+`)
+
if tok == arvadostest.ActiveToken {
c.Check(code, check.Equals, http.StatusOK)
c.Check(body, check.Equals, "foo")
+
} else {
c.Check(code >= 400, check.Equals, true)
c.Check(code < 500, check.Equals, true)
@@ -151,11 +173,11 @@ func doVhostRequestsWithHostPath(c *check.C, authz authorizer, hostPath string)
}
}
-func doReq(req *http.Request) *httptest.ResponseRecorder {
+func doReq(req *http.Request) (*http.Request, *httptest.ResponseRecorder) {
resp := httptest.NewRecorder()
(&handler{}).ServeHTTP(resp, req)
if resp.Code != http.StatusSeeOther {
- return resp
+ return req, resp
}
cookies := (&http.Response{Header: resp.Header()}).Cookies()
u, _ := req.URL.Parse(resp.Header().Get("Location"))
@@ -376,6 +398,34 @@ func (s *IntegrationSuite) TestRange(c *check.C) {
}
}
+// XHRs can't follow redirect-with-cookie so they rely on method=POST
+// and disposition=attachment (telling us it's acceptable to respond
+// with content instead of a redirect) and an Origin header that gets
+// added automatically by the browser (telling us it's desirable to do
+// so).
+func (s *IntegrationSuite) TestXHRNoRedirect(c *check.C) {
+ u, _ := url.Parse("http://example.com/c=" + arvadostest.FooCollection + "/foo")
+ req := &http.Request{
+ Method: "POST",
+ Host: u.Host,
+ URL: u,
+ RequestURI: u.RequestURI(),
+ Header: http.Header{
+ "Origin": {"https://origin.example"},
+ "Content-Type": {"application/x-www-form-urlencoded"},
+ },
+ Body: ioutil.NopCloser(strings.NewReader(url.Values{
+ "api_token": {arvadostest.ActiveToken},
+ "disposition": {"attachment"},
+ }.Encode())),
+ }
+ resp := httptest.NewRecorder()
+ (&handler{}).ServeHTTP(resp, req)
+ c.Check(resp.Code, check.Equals, http.StatusOK)
+ c.Check(resp.Body.String(), check.Equals, "foo")
+ c.Check(resp.Header().Get("Access-Control-Allow-Origin"), check.Equals, "*")
+}
+
func (s *IntegrationSuite) testVhostRedirectTokenToCookie(c *check.C, method, hostPath, queryString, contentType, reqBody string, expectStatus int, expectRespBody string) *httptest.ResponseRecorder {
u, _ := url.Parse(`http://` + hostPath + queryString)
req := &http.Request{
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list