[ARVADOS] updated: 384a51756fac8fffa2f600d78ff62d0652a8b1e6
git at public.curoverse.com
git at public.curoverse.com
Thu Dec 17 01:34:05 EST 2015
Summary of changes:
services/keep-web/handler.go | 1 +
services/keep-web/handler_test.go | 15 +++++++++------
2 files changed, 10 insertions(+), 6 deletions(-)
discards b59ac41bf409566da73e43ecf4927b40729fc72c (commit)
discards fa1bb089c55056fc25cf888b11b43b3a2ac2feca (commit)
via 384a51756fac8fffa2f600d78ff62d0652a8b1e6 (commit)
via a55449212d00b5d6c52c2bc2df1f0c57175a47e1 (commit)
This update added new revisions after undoing existing revisions. That is
to say, the old revision is not a strict subset of the new revision. This
situation occurs when you --force push a change and generate a repository
containing something like this:
* -- * -- B -- O -- O -- O (b59ac41bf409566da73e43ecf4927b40729fc72c)
\
N -- N -- N (384a51756fac8fffa2f600d78ff62d0652a8b1e6)
When this happens we assume that you've already had alert emails for all
of the O revisions, and so we here report only the revisions in the N
branch from the common base, B.
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 384a51756fac8fffa2f600d78ff62d0652a8b1e6
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 a55449212d00b5d6c52c2bc2df1f0c57175a47e1
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