[ARVADOS] updated: 7aced94c664f1168f53c914630d41410e64d9f31

git at public.curoverse.com git at public.curoverse.com
Mon Jan 18 16:18:02 EST 2016


Summary of changes:
 apps/workbench/app/controllers/collections_controller.rb | 1 +
 services/keep-web/doc.go                                 | 8 +++++---
 2 files changed, 6 insertions(+), 3 deletions(-)

  discards  83f613777343c12cc1e76209f4f509895b3a941b (commit)
       via  7aced94c664f1168f53c914630d41410e64d9f31 (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 (83f613777343c12cc1e76209f4f509895b3a941b)
            \
             N -- N -- N (7aced94c664f1168f53c914630d41410e64d9f31)

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 7aced94c664f1168f53c914630d41410e64d9f31
Author: Tom Clegg <tom at curoverse.com>
Date:   Mon Jan 18 16:17:17 2016 -0500

    8177: Expand trust_all_content comments.
    
    Skip whole check_uri block when trust_all_content.
    
    Fix test name.

diff --git a/apps/workbench/app/controllers/collections_controller.rb b/apps/workbench/app/controllers/collections_controller.rb
index 63af828..7a00242 100644
--- a/apps/workbench/app/controllers/collections_controller.rb
+++ b/apps/workbench/app/controllers/collections_controller.rb
@@ -339,7 +339,7 @@ class CollectionsController < ApplicationController
       # Prefer the attachment-only-host when we want an attachment
       # (and when there is no preview link configured)
       tmpl = Rails.configuration.keep_web_download_url
-    else
+    elsif not Rails.configuration.trust_all_content
       check_uri = URI.parse(tmpl % fmt)
       if opts[:query_token] and
           not check_uri.host.start_with?(munged_id + "--") and
@@ -347,9 +347,7 @@ class CollectionsController < ApplicationController
         # We're about to pass a token in the query string, but
         # keep-web can't accept that safely at a single-origin URL
         # template (unless it's -attachment-only-host).
-        unless (Rails.configuration.trust_all_content and tmpl)
-          tmpl = Rails.configuration.keep_web_download_url
-        end
+        tmpl = Rails.configuration.keep_web_download_url
         if not tmpl
           raise ArgumentError, "Download precluded by site configuration"
         end
diff --git a/apps/workbench/config/application.default.yml b/apps/workbench/config/application.default.yml
index 7e8c3aa..239ffcd 100644
--- a/apps/workbench/config/application.default.yml
+++ b/apps/workbench/config/application.default.yml
@@ -258,9 +258,17 @@ common:
   # keep_web_download_url: https://download.uuid_prefix.arvadosapi.com/c=%{uuid_or_pdh}
   keep_web_download_url: false
 
-  # In "trust all content" mode, Workbench will redirect users to
-  # keep-web even when that exposes XSS vulnerabilities.
+  # In "trust all content" mode, Workbench will redirect download
+  # requests to keep-web, even in the cases when keep-web would have
+  # to expose XSS vulnerabilities in order to handle the redirect.
   #
-  # When enabling this setting, the corresponding setting on the
-  # keep-web server must also be enabled.
+  # When enabling this setting, the -trust-all-content flag on the
+  # keep-web server must also be enabled.  For more detail, see
+  # https://godoc.org/github.com/curoverse/arvados/services/keep-web
+  #
+  # This setting has no effect in the recommended configuration, where
+  # the host part of keep_web_url begins with %{uuid_or_pdh}: in this
+  # case XSS protection is provided by browsers' same-origin policy.
+  #
+  # The default setting (false) is appropriate for a multi-user site.
   trust_all_content: false
diff --git a/apps/workbench/test/controllers/collections_controller_test.rb b/apps/workbench/test/controllers/collections_controller_test.rb
index 0cd747e..45aab3c 100644
--- a/apps/workbench/test/controllers/collections_controller_test.rb
+++ b/apps/workbench/test/controllers/collections_controller_test.rb
@@ -578,7 +578,7 @@ class CollectionsControllerTest < ActionController::TestCase
       assert_equal "https://download.example/c=#{id.sub '+', '-'}/_/w%20a%20z?api_token=#{tok}", @response.redirect_url
     end
 
-    test "Redirect to keep_web_download_url via #{id_type} when trust_all_content enabled" do
+    test "Redirect to keep_web_url via #{id_type} when trust_all_content enabled" do
       Rails.configuration.trust_all_content = true
       setup_for_keep_web('https://collections.example/c=%{uuid_or_pdh}',
                          'https://download.example/c=%{uuid_or_pdh}')
diff --git a/services/keep-web/doc.go b/services/keep-web/doc.go
index 5a66d86..9ca732f 100644
--- a/services/keep-web/doc.go
+++ b/services/keep-web/doc.go
@@ -217,7 +217,7 @@
 //
 // In "trust all content" mode, Keep-web will accept credentials (API
 // tokens) and serve any collection X at
-// "https://collections.example.com/collections/X/path/file.ext".
+// "https://collections.example.com/c=X/path/file.ext".
 // This is UNSAFE except in the special case where everyone who is
 // able write ANY data to Keep, and every JavaScript and HTML file
 // written to Keep, is also trusted to read ALL of the data in Keep.
@@ -233,6 +233,8 @@
 //   keep-web -listen :9999 -attachment-only-host domain.example:9999 -trust-all-content
 //
 // Depending on your site configuration, you might also want to enable
-// "trust all content" setting on Workbench, in which case Workbench will
-// redirect users to keep-web even when that exposes XSS vulnerabilities.
+// "trust all content" setting on Workbench. Normally, Workbench
+// avoids redirecting requests to keep-web if they depend on
+// -trust-all-content being set.
+//
 package main

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list