17344: Merge branch 'main'
authorTom Clegg <tom@curii.com>
Wed, 20 Jul 2022 19:02:00 +0000 (15:02 -0400)
committerTom Clegg <tom@curii.com>
Wed, 20 Jul 2022 19:02:00 +0000 (15:02 -0400)
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

13 files changed:
apps/workbench/Gemfile.lock
cmd/arvados-package/install.go
doc/install/automatic.html.textile.liquid
lib/boot/cert.go
lib/config/config.default.yml
lib/install/init.go
lib/service/tls.go
sdk/go/arvados/config.go
services/api/Gemfile.lock
services/api/app/models/user.rb
services/api/test/integration/users_test.rb
services/keep-web/handler.go
services/keep-web/handler_test.go

index ebb4b64c607314606f9fb8c4c54e092ff7be1353..1102ca10337efbb75adfcfe1c498cfc47082c198 100644 (file)
@@ -16,43 +16,43 @@ GEM
   remote: https://rubygems.org/
   specs:
     RedCloth (4.3.2)
-    actioncable (5.2.8)
-      actionpack (= 5.2.8)
+    actioncable (5.2.8.1)
+      actionpack (= 5.2.8.1)
       nio4r (~> 2.0)
       websocket-driver (>= 0.6.1)
-    actionmailer (5.2.8)
-      actionpack (= 5.2.8)
-      actionview (= 5.2.8)
-      activejob (= 5.2.8)
+    actionmailer (5.2.8.1)
+      actionpack (= 5.2.8.1)
+      actionview (= 5.2.8.1)
+      activejob (= 5.2.8.1)
       mail (~> 2.5, >= 2.5.4)
       rails-dom-testing (~> 2.0)
-    actionpack (5.2.8)
-      actionview (= 5.2.8)
-      activesupport (= 5.2.8)
+    actionpack (5.2.8.1)
+      actionview (= 5.2.8.1)
+      activesupport (= 5.2.8.1)
       rack (~> 2.0, >= 2.0.8)
       rack-test (>= 0.6.3)
       rails-dom-testing (~> 2.0)
       rails-html-sanitizer (~> 1.0, >= 1.0.2)
-    actionview (5.2.8)
-      activesupport (= 5.2.8)
+    actionview (5.2.8.1)
+      activesupport (= 5.2.8.1)
       builder (~> 3.1)
       erubi (~> 1.4)
       rails-dom-testing (~> 2.0)
       rails-html-sanitizer (~> 1.0, >= 1.0.3)
-    activejob (5.2.8)
-      activesupport (= 5.2.8)
+    activejob (5.2.8.1)
+      activesupport (= 5.2.8.1)
       globalid (>= 0.3.6)
-    activemodel (5.2.8)
-      activesupport (= 5.2.8)
-    activerecord (5.2.8)
-      activemodel (= 5.2.8)
-      activesupport (= 5.2.8)
+    activemodel (5.2.8.1)
+      activesupport (= 5.2.8.1)
+    activerecord (5.2.8.1)
+      activemodel (= 5.2.8.1)
+      activesupport (= 5.2.8.1)
       arel (>= 9.0)
-    activestorage (5.2.8)
-      actionpack (= 5.2.8)
-      activerecord (= 5.2.8)
+    activestorage (5.2.8.1)
+      actionpack (= 5.2.8.1)
+      activerecord (= 5.2.8.1)
       marcel (~> 1.0.0)
-    activesupport (5.2.8)
+    activesupport (5.2.8.1)
       concurrent-ruby (~> 1.0, >= 1.0.2)
       i18n (>= 0.7, < 2)
       minitest (~> 5.1)
@@ -179,7 +179,7 @@ GEM
     net-ssh-gateway (2.0.0)
       net-ssh (>= 4.0.0)
     nio4r (2.5.8)
-    nokogiri (1.13.6)
+    nokogiri (1.13.7)
       mini_portile2 (~> 2.8.0)
       racc (~> 1.4)
     npm-rails (0.2.1)
@@ -200,23 +200,23 @@ GEM
       websocket-driver (>= 0.2.0)
     public_suffix (4.0.6)
     racc (1.6.0)
-    rack (2.2.3.1)
+    rack (2.2.4)
     rack-mini-profiler (1.0.2)
       rack (>= 1.2.0)
-    rack-test (1.1.0)
-      rack (>= 1.0, < 3)
-    rails (5.2.8)
-      actioncable (= 5.2.8)
-      actionmailer (= 5.2.8)
-      actionpack (= 5.2.8)
-      actionview (= 5.2.8)
-      activejob (= 5.2.8)
-      activemodel (= 5.2.8)
-      activerecord (= 5.2.8)
-      activestorage (= 5.2.8)
-      activesupport (= 5.2.8)
+    rack-test (2.0.2)
+      rack (>= 1.3)
+    rails (5.2.8.1)
+      actioncable (= 5.2.8.1)
+      actionmailer (= 5.2.8.1)
+      actionpack (= 5.2.8.1)
+      actionview (= 5.2.8.1)
+      activejob (= 5.2.8.1)
+      activemodel (= 5.2.8.1)
+      activerecord (= 5.2.8.1)
+      activestorage (= 5.2.8.1)
+      activesupport (= 5.2.8.1)
       bundler (>= 1.3.0)
-      railties (= 5.2.8)
+      railties (= 5.2.8.1)
       sprockets-rails (>= 2.0.0)
     rails-controller-testing (1.0.4)
       actionpack (>= 5.0.1.x)
@@ -228,9 +228,9 @@ GEM
     rails-html-sanitizer (1.4.3)
       loofah (~> 2.3)
     rails-perftest (0.0.7)
-    railties (5.2.8)
-      actionpack (= 5.2.8)
-      activesupport (= 5.2.8)
+    railties (5.2.8.1)
+      actionpack (= 5.2.8.1)
+      activesupport (= 5.2.8.1)
       method_source
       rake (>= 0.8.7)
       thor (>= 0.19.0, < 2.0)
index 9273ac9c73e4850f18d580899dfeae72b2719bd9..38efae0461dba43593332f353e48f883b006c6c9 100644 (file)
@@ -92,7 +92,7 @@ rm /etc/apt/sources.list.d/arvados-local.list
        if opts.Live != "" {
                cmd.Args = append(cmd.Args,
                        "--env=domain="+opts.Live,
-                       "--env=initargs=-tls=acme",
+                       "--env=initargs=-tls=/var/lib/acme/live/"+opts.Live,
                        "--env=bootargs=",
                        "--publish=:443:443",
                        "--publish=:4440-4460:4440-4460",
index 33c6fd3d3795d1bb886eff22d49a904121f9a029..398ebc20e024c745f4ba49a0b5ecab260ce215e8 100644 (file)
@@ -53,7 +53,7 @@ h2. Initialize the cluster
 # echo > /etc/apt/sources.list.d/arvados.list "deb http://apt.arvados.org/$(lsb_release -sc) $(lsb_release -sc) main"
 # apt update
 # apt install arvados-server-easy
-# arvados-server init -cluster-id x9999 -domain x9999.example.com -tls auto -login pam
+# arvados-server init -cluster-id x9999 -domain x9999.example.com -tls acme -login pam
 </pre>
 
 When the "init" command is finished, navigate to the link shown in the terminal (e.g., @https://x9999.example.com/@) and log in with the account you created above.
index 10fd0aa9f6bc90e0b014145e25bb68d1d496fe9a..175a35080373ae33b56e7b79be14aa7774e38167 100644 (file)
@@ -36,7 +36,7 @@ func (createCertificates) String() string {
 }
 
 func (createCertificates) Run(ctx context.Context, fail func(error), super *Supervisor) error {
-       if super.cluster.TLS.Automatic {
+       if super.cluster.TLS.ACME.Server != "" {
                return bootAutoCert(ctx, fail, super)
        } else if super.cluster.TLS.Key == "" && super.cluster.TLS.Certificate == "" {
                return createSelfSignedCert(ctx, fail, super)
@@ -78,8 +78,15 @@ func bootAutoCert(ctx context.Context, fail func(error), super *Supervisor) erro
                        }
                },
        }
-       if super.cluster.TLS.Staging {
+       if srv := super.cluster.TLS.ACME.Server; srv == "LE" {
+               // Leaving mgr.Client == nil means use Let's Encrypt
+               // production environment
+       } else if srv == "LE-staging" {
                mgr.Client = &acme.Client{DirectoryURL: stagingDirectoryURL}
+       } else if strings.HasPrefix(srv, "https://") {
+               mgr.Client = &acme.Client{DirectoryURL: srv}
+       } else {
+               return fmt.Errorf("autocert setup: invalid directory URL in TLS.ACME.Server: %q", srv)
        }
        go func() {
                err := http.ListenAndServe(":80", mgr.HTTPHandler(nil))
index c8c02cc6040b248bef2506ac11075767f2efa95e..b23c6a12745088fc02c65cb670fc0a1936e30a45 100644 (file)
@@ -900,8 +900,8 @@ Clusters:
       Repositories: /var/lib/arvados/git/repositories
 
     TLS:
-      # Use "file:///var/lib/acme/live/example.com/cert" and ".../key"
-      # to load externally managed certificates.
+      # Use "file:///var/lib/acme/live/example.com/cert" and
+      # ".../privkey" to load externally managed certificates.
       Certificate: ""
       Key: ""
 
@@ -909,17 +909,21 @@ Clusters:
       # use this in production.
       Insecure: false
 
-      # Agree to Let's Encrypt terms of service and obtain
-      # certificates automatically for ExternalURL domains.
-      #
-      # Note: this feature is not yet implemented in released
-      # versions, only in the alpha/prerelease arvados-server-easy
-      # package.
-      Automatic: false
-
-      # Use Let's Encrypt staging environment instead of production
-      # environment.
-      Staging: false
+      ACME:
+        # Obtain certificates automatically for ExternalURL domains
+        # using an ACME server and http-01 validation.
+        #
+        # To use Let's Encrypt, specify "LE".  To use the Let's
+        # Encrypt staging environment, specify "LE-staging".  To use a
+        # different ACME server, specify the full directory URL
+        # ("https://...").
+        #
+        # Note: this feature is not yet implemented in released
+        # versions, only in the alpha/prerelease arvados-server-easy
+        # package.
+        #
+        # Implies agreement with the server's terms of service.
+        Server: ""
 
     Containers:
       # List of supported Docker Registry image formats that compute nodes
index 14e2eaafab85079876f32bccf9de110fdd4f6373..79c9fbdac71de4c616d2134550999c93ccf5de34 100644 (file)
@@ -51,6 +51,7 @@ type initCommand struct {
        LoginGoogle             bool
        LoginGoogleClientID     string
        LoginGoogleClientSecret string
+       TLSDir                  string
 }
 
 func (initcmd *initCommand) RunCommand(prog string, args []string, stdin io.Reader, stdout, stderr io.Writer) int {
@@ -79,7 +80,7 @@ func (initcmd *initCommand) RunCommand(prog string, args []string, stdin io.Read
        flags.StringVar(&initcmd.Domain, "domain", hostname, "cluster public DNS `name`, like x1234.arvadosapi.com")
        flags.StringVar(&initcmd.Login, "login", "", "login `backend`: test, pam, 'google {client-id} {client-secret}', or ''")
        flags.StringVar(&initcmd.AdminEmail, "admin-email", "", "give admin privileges to user with given `email`")
-       flags.StringVar(&initcmd.TLS, "tls", "none", "tls certificate `source`: acme, auto, insecure, or none")
+       flags.StringVar(&initcmd.TLS, "tls", "none", "tls certificate `source`: acme, insecure, none, or /path/to/dir containing privkey and cert files")
        flags.BoolVar(&initcmd.Start, "start", true, "start systemd service after creating config")
        if ok, code := cmd.ParseFlags(flags, prog, args, "", stderr); !ok {
                return code
@@ -108,6 +109,16 @@ func (initcmd *initCommand) RunCommand(prog string, args []string, stdin io.Read
                return 1
        }
 
+       switch initcmd.TLS {
+       case "none", "acme", "insecure":
+       default:
+               if !strings.HasPrefix(initcmd.TLS, "/") {
+                       err = fmt.Errorf("invalid argument to -tls: %q; see %s -help", initcmd.TLS, prog)
+                       return 1
+               }
+               initcmd.TLSDir = initcmd.TLS
+       }
+
        confdir := "/etc/arvados"
        conffile := confdir + "/config.yml"
        if _, err = os.Stat(conffile); err == nil {
@@ -254,11 +265,12 @@ func (initcmd *initCommand) RunCommand(prog string, args []string, stdin io.Read
     TLS:
       {{if eq .TLS "insecure"}}
       Insecure: true
-      {{else if eq .TLS "auto"}}
-      Automatic: true
       {{else if eq .TLS "acme"}}
-      Certificate: {{printf "%q" (print "/var/lib/acme/live/" .Domain "/cert")}}
-      Key: {{printf "%q" (print "/var/lib/acme/live/" .Domain "/privkey")}}
+      ACME:
+        Server: LE
+      {{else if ne .TLSDir ""}}
+      Certificate: {{printf "%q" (print .TLSDir "/cert")}}
+      Key: {{printf "%q" (print .TLSDir "/privkey")}}
       {{else}}
       {}
       {{end}}
index 234ee5787855c53929af5885590fb1abdeaece80..88a2858beb13e2f5db216a7645423fd7d2c7c541 100644 (file)
@@ -21,7 +21,7 @@ import (
 )
 
 func makeTLSConfig(cluster *arvados.Cluster, logger logrus.FieldLogger) (*tls.Config, error) {
-       if cluster.TLS.Automatic {
+       if cluster.TLS.ACME.Server != "" {
                return makeAutocertConfig(cluster, logger)
        } else {
                return makeFileLoaderConfig(cluster, logger)
index d9aa92b65d0fbfe1c97da212f5b1661e92e381be..6d8f39dfb316fbaba1cf64f71ef5f5f778f91e8e 100644 (file)
@@ -227,8 +227,9 @@ type Cluster struct {
                Certificate string
                Key         string
                Insecure    bool
-               Automatic   bool
-               Staging     bool
+               ACME        struct {
+                       Server string
+               }
        }
        Users struct {
                ActivatedUsersAreVisibleToOthers      bool
index 49fa58493557a45584a59c992b14e595492ad935..bc6215496fab309ce2ae0668ef1a7f56f526bec9 100644 (file)
@@ -8,43 +8,43 @@ GIT
 GEM
   remote: https://rubygems.org/
   specs:
-    actioncable (5.2.8)
-      actionpack (= 5.2.8)
+    actioncable (5.2.8.1)
+      actionpack (= 5.2.8.1)
       nio4r (~> 2.0)
       websocket-driver (>= 0.6.1)
-    actionmailer (5.2.8)
-      actionpack (= 5.2.8)
-      actionview (= 5.2.8)
-      activejob (= 5.2.8)
+    actionmailer (5.2.8.1)
+      actionpack (= 5.2.8.1)
+      actionview (= 5.2.8.1)
+      activejob (= 5.2.8.1)
       mail (~> 2.5, >= 2.5.4)
       rails-dom-testing (~> 2.0)
-    actionpack (5.2.8)
-      actionview (= 5.2.8)
-      activesupport (= 5.2.8)
+    actionpack (5.2.8.1)
+      actionview (= 5.2.8.1)
+      activesupport (= 5.2.8.1)
       rack (~> 2.0, >= 2.0.8)
       rack-test (>= 0.6.3)
       rails-dom-testing (~> 2.0)
       rails-html-sanitizer (~> 1.0, >= 1.0.2)
-    actionview (5.2.8)
-      activesupport (= 5.2.8)
+    actionview (5.2.8.1)
+      activesupport (= 5.2.8.1)
       builder (~> 3.1)
       erubi (~> 1.4)
       rails-dom-testing (~> 2.0)
       rails-html-sanitizer (~> 1.0, >= 1.0.3)
-    activejob (5.2.8)
-      activesupport (= 5.2.8)
+    activejob (5.2.8.1)
+      activesupport (= 5.2.8.1)
       globalid (>= 0.3.6)
-    activemodel (5.2.8)
-      activesupport (= 5.2.8)
-    activerecord (5.2.8)
-      activemodel (= 5.2.8)
-      activesupport (= 5.2.8)
+    activemodel (5.2.8.1)
+      activesupport (= 5.2.8.1)
+    activerecord (5.2.8.1)
+      activemodel (= 5.2.8.1)
+      activesupport (= 5.2.8.1)
       arel (>= 9.0)
-    activestorage (5.2.8)
-      actionpack (= 5.2.8)
-      activerecord (= 5.2.8)
+    activestorage (5.2.8.1)
+      actionpack (= 5.2.8.1)
+      activerecord (= 5.2.8.1)
       marcel (~> 1.0.0)
-    activesupport (5.2.8)
+    activesupport (5.2.8.1)
       concurrent-ruby (~> 1.0, >= 1.0.2)
       i18n (>= 0.7, < 2)
       minitest (~> 5.1)
@@ -140,7 +140,7 @@ GEM
     multi_json (1.15.0)
     multipart-post (2.1.1)
     nio4r (2.5.8)
-    nokogiri (1.13.6)
+    nokogiri (1.13.7)
       mini_portile2 (~> 2.8.0)
       racc (~> 1.4)
     oj (3.9.2)
@@ -153,21 +153,21 @@ GEM
     power_assert (1.1.4)
     public_suffix (4.0.6)
     racc (1.6.0)
-    rack (2.2.3.1)
-    rack-test (1.1.0)
-      rack (>= 1.0, < 3)
-    rails (5.2.8)
-      actioncable (= 5.2.8)
-      actionmailer (= 5.2.8)
-      actionpack (= 5.2.8)
-      actionview (= 5.2.8)
-      activejob (= 5.2.8)
-      activemodel (= 5.2.8)
-      activerecord (= 5.2.8)
-      activestorage (= 5.2.8)
-      activesupport (= 5.2.8)
+    rack (2.2.4)
+    rack-test (2.0.2)
+      rack (>= 1.3)
+    rails (5.2.8.1)
+      actioncable (= 5.2.8.1)
+      actionmailer (= 5.2.8.1)
+      actionpack (= 5.2.8.1)
+      actionview (= 5.2.8.1)
+      activejob (= 5.2.8.1)
+      activemodel (= 5.2.8.1)
+      activerecord (= 5.2.8.1)
+      activestorage (= 5.2.8.1)
+      activesupport (= 5.2.8.1)
       bundler (>= 1.3.0)
-      railties (= 5.2.8)
+      railties (= 5.2.8.1)
       sprockets-rails (>= 2.0.0)
     rails-controller-testing (1.0.4)
       actionpack (>= 5.0.1.x)
@@ -181,9 +181,9 @@ GEM
     rails-observers (0.1.5)
       activemodel (>= 4.0)
     rails-perftest (0.0.7)
-    railties (5.2.8)
-      actionpack (= 5.2.8)
-      activesupport (= 5.2.8)
+    railties (5.2.8.1)
+      actionpack (= 5.2.8.1)
+      activesupport (= 5.2.8.1)
       method_source
       rake (>= 0.8.7)
       thor (>= 0.19.0, < 2.0)
index 52d36ac57735f0c16d4b0ed6271a50681e08e05b..1662278cc3edc0c4b9c681d51b1696c195b27a98 100644 (file)
@@ -24,6 +24,7 @@ class User < ArvadosModel
   validate :identity_url_nil_if_empty
   before_update :prevent_privilege_escalation
   before_update :prevent_inactive_admin
+  before_update :prevent_nonadmin_system_root
   before_update :verify_repositories_empty, :if => Proc.new {
     username.nil? and username_changed?
   }
@@ -301,6 +302,10 @@ SELECT target_uuid, perm_level
 
   # delete user signatures, login, repo, and vm perms, and mark as inactive
   def unsetup
+    if self.uuid == system_user_uuid
+      raise "System root user cannot be deactivated"
+    end
+
     # delete oid_login_perms for this user
     #
     # note: these permission links are obsolete, they have no effect
@@ -345,6 +350,11 @@ SELECT target_uuid, perm_level
     self.save!
   end
 
+  # Called from ArvadosModel
+  def set_default_owner
+    self.owner_uuid = system_user_uuid
+  end
+
   def must_unsetup_to_deactivate
     if !self.new_record? &&
        self.uuid[0..4] == Rails.configuration.Login.LoginCluster &&
@@ -702,6 +712,13 @@ SELECT target_uuid, perm_level
     true
   end
 
+  def prevent_nonadmin_system_root
+    if self.uuid == system_user_uuid and self.is_admin_changed? and !self.is_admin
+      raise "System root user cannot be non-admin"
+    end
+    true
+  end
+
   def search_permissions(start, graph, merged={}, upstream_mask=nil, upstream_path={})
     nextpaths = graph[start]
     return merged if !nextpaths
index f3e787e3dffc6ed01570b249865068f1ecfacb7e..430f0d385d7e3789995af57219ece58eeec59367 100644 (file)
@@ -480,4 +480,60 @@ class UsersTest < ActionDispatch::IntegrationTest
     assert_response 403
   end
 
+  test "disabling system root user not permitted" do
+    put("/arvados/v1/users/#{users(:system_user).uuid}",
+      params: {
+        user: {is_admin: false}
+      },
+      headers: auth(:admin))
+    assert_response 422
+
+    post("/arvados/v1/users/#{users(:system_user).uuid}/unsetup",
+      params: {},
+      headers: auth(:admin))
+    assert_response 422
+  end
+
+  test "creating users only accepted for admins" do
+    assert_equal false, users(:active).is_admin
+    post '/arvados/v1/users',
+      params: {
+        "user" => {
+          "email" => 'foo@example.com',
+          "username" => "barney"
+        }
+      },
+      headers: auth(:active)
+    assert_response 403
+  end
+
+  test "create users assigns the system root user as their owner" do
+    post '/arvados/v1/users',
+      params: {
+        "user" => {
+          "email" => 'foo@example.com',
+          "username" => "barney"
+        }
+      },
+      headers: auth(:admin)
+    assert_response :success
+    assert_not_nil json_response["uuid"]
+    assert_equal users(:system_user).uuid, json_response["owner_uuid"]
+  end
+
+  test "create users ignores provided owner_uuid field" do
+    assert_equal false, users(:admin).uuid == users(:system_user).uuid
+    post '/arvados/v1/users',
+      params: {
+        "user" => {
+          "email" => 'foo@example.com',
+          "owner_uuid" => users(:admin).uuid,
+          "username" => "barney"
+        }
+      },
+      headers: auth(:admin)
+    assert_response :success
+    assert_not_nil json_response["uuid"]
+    assert_equal users(:system_user).uuid, json_response["owner_uuid"]
+  end
 end
index 54b8c02165e79c03e8a23410b80691e0561e7688..1f1f509860bb9950d95e5d9c566e9e57f9d4df36 100644 (file)
@@ -411,16 +411,44 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
                        }
                }
                // The client's token was invalid (e.g., expired), or
-               // the client didn't even provide one.  Propagate the
-               // 401 to encourage the client to use a [different]
-               // token.
+               // the client didn't even provide one.  Redirect to
+               // workbench2's login-and-redirect-to-download url if
+               // this is a browser navigation request. (The redirect
+               // flow can't preserve the original method if it's not
+               // GET, and doesn't make sense if the UA is a
+               // command-line tool, is trying to load an inline
+               // image, etc.; in these cases, there's nothing we can
+               // do, so return 401 unauthorized.)
+               //
+               // Note Sec-Fetch-Mode is sent by all non-EOL
+               // browsers, except Safari.
+               // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Sec-Fetch-Mode
                //
                // TODO(TC): This response would be confusing to
                // someone trying (anonymously) to download public
                // data that has been deleted.  Allow a referrer to
                // provide this context somehow?
-               w.Header().Add("WWW-Authenticate", "Basic realm=\"collections\"")
-               http.Error(w, unauthorizedMessage, http.StatusUnauthorized)
+               if r.Method == http.MethodGet && r.Header.Get("Sec-Fetch-Mode") == "navigate" {
+                       target := url.URL(h.Cluster.Services.Workbench2.ExternalURL)
+                       redirkey := "redirectToPreview"
+                       if attachment {
+                               redirkey = "redirectToDownload"
+                       }
+                       callback := "/c=" + collectionID + "/" + strings.Join(targetPath, "/")
+                       // target.RawQuery = url.Values{redirkey:
+                       // {target}}.Encode() would be the obvious
+                       // thing to do here, but wb2 doesn't decode
+                       // this as a query param -- it takes
+                       // everything after "${redirkey}=" as the
+                       // target URL. If we encode "/" as "%2F" etc.,
+                       // the redirect won't work.
+                       target.RawQuery = redirkey + "=" + callback
+                       w.Header().Add("Location", target.String())
+                       w.WriteHeader(http.StatusSeeOther)
+               } else {
+                       w.Header().Add("WWW-Authenticate", "Basic realm=\"collections\"")
+                       http.Error(w, unauthorizedMessage, http.StatusUnauthorized)
+               }
                return
        }
 
index 92fea87a01c0eec63ce53162a3d61f0634ebba37..768013185ae7d50dc450fecd342ebcdc343db059 100644 (file)
@@ -391,7 +391,7 @@ func (s *IntegrationSuite) TestVhostRedirectQueryTokenToCookie(c *check.C) {
        s.testVhostRedirectTokenToCookie(c, "GET",
                arvadostest.FooCollection+".example.com/foo",
                "?api_token="+arvadostest.ActiveToken,
-               "",
+               nil,
                "",
                http.StatusOK,
                "foo",
@@ -402,7 +402,7 @@ func (s *IntegrationSuite) TestSingleOriginSecretLink(c *check.C) {
        s.testVhostRedirectTokenToCookie(c, "GET",
                "example.com/c="+arvadostest.FooCollection+"/t="+arvadostest.ActiveToken+"/foo",
                "",
-               "",
+               nil,
                "",
                http.StatusOK,
                "foo",
@@ -415,7 +415,7 @@ func (s *IntegrationSuite) TestSingleOriginSecretLinkBadToken(c *check.C) {
        s.testVhostRedirectTokenToCookie(c, "GET",
                "example.com/c="+arvadostest.FooCollection+"/t=bogus/foo",
                "",
-               "",
+               nil,
                "",
                http.StatusNotFound,
                notFoundMessage+"\n",
@@ -423,13 +423,70 @@ func (s *IntegrationSuite) TestSingleOriginSecretLinkBadToken(c *check.C) {
 }
 
 // Bad token in a cookie (even if it got there via our own
-// query-string-to-cookie redirect) is, in principle, retryable at the
-// same URL so it's 401 Unauthorized.
+// query-string-to-cookie redirect) is, in principle, retryable via
+// wb2-login-and-redirect flow.
 func (s *IntegrationSuite) TestVhostRedirectQueryTokenToBogusCookie(c *check.C) {
-       s.testVhostRedirectTokenToCookie(c, "GET",
+       // Inline
+       resp := s.testVhostRedirectTokenToCookie(c, "GET",
+               arvadostest.FooCollection+".example.com/foo",
+               "?api_token=thisisabogustoken",
+               http.Header{"Sec-Fetch-Mode": {"navigate"}},
+               "",
+               http.StatusSeeOther,
+               "",
+       )
+       u, err := url.Parse(resp.Header().Get("Location"))
+       c.Assert(err, check.IsNil)
+       c.Logf("redirected to %s", u)
+       c.Check(u.Host, check.Equals, s.handler.Cluster.Services.Workbench2.ExternalURL.Host)
+       c.Check(u.Query().Get("redirectToPreview"), check.Equals, "/c="+arvadostest.FooCollection+"/foo")
+       c.Check(u.Query().Get("redirectToDownload"), check.Equals, "")
+
+       // Download/attachment indicated by ?disposition=attachment
+       resp = s.testVhostRedirectTokenToCookie(c, "GET",
                arvadostest.FooCollection+".example.com/foo",
+               "?api_token=thisisabogustoken&disposition=attachment",
+               http.Header{"Sec-Fetch-Mode": {"navigate"}},
+               "",
+               http.StatusSeeOther,
+               "",
+       )
+       u, err = url.Parse(resp.Header().Get("Location"))
+       c.Assert(err, check.IsNil)
+       c.Logf("redirected to %s", u)
+       c.Check(u.Host, check.Equals, s.handler.Cluster.Services.Workbench2.ExternalURL.Host)
+       c.Check(u.Query().Get("redirectToPreview"), check.Equals, "")
+       c.Check(u.Query().Get("redirectToDownload"), check.Equals, "/c="+arvadostest.FooCollection+"/foo")
+
+       // Download/attachment indicated by vhost
+       resp = s.testVhostRedirectTokenToCookie(c, "GET",
+               s.handler.Cluster.Services.WebDAVDownload.ExternalURL.Host+"/c="+arvadostest.FooCollection+"/foo",
                "?api_token=thisisabogustoken",
+               http.Header{"Sec-Fetch-Mode": {"navigate"}},
                "",
+               http.StatusSeeOther,
+               "",
+       )
+       u, err = url.Parse(resp.Header().Get("Location"))
+       c.Assert(err, check.IsNil)
+       c.Logf("redirected to %s", u)
+       c.Check(u.Host, check.Equals, s.handler.Cluster.Services.Workbench2.ExternalURL.Host)
+       c.Check(u.Query().Get("redirectToPreview"), check.Equals, "")
+       c.Check(u.Query().Get("redirectToDownload"), check.Equals, "/c="+arvadostest.FooCollection+"/foo")
+
+       // Without "Sec-Fetch-Mode: navigate" header, just 401.
+       s.testVhostRedirectTokenToCookie(c, "GET",
+               s.handler.Cluster.Services.WebDAVDownload.ExternalURL.Host+"/c="+arvadostest.FooCollection+"/foo",
+               "?api_token=thisisabogustoken",
+               http.Header{"Sec-Fetch-Mode": {"cors"}},
+               "",
+               http.StatusUnauthorized,
+               unauthorizedMessage+"\n",
+       )
+       s.testVhostRedirectTokenToCookie(c, "GET",
+               s.handler.Cluster.Services.WebDAVDownload.ExternalURL.Host+"/c="+arvadostest.FooCollection+"/foo",
+               "?api_token=thisisabogustoken",
+               nil,
                "",
                http.StatusUnauthorized,
                unauthorizedMessage+"\n",
@@ -440,7 +497,7 @@ func (s *IntegrationSuite) TestVhostRedirectQueryTokenSingleOriginError(c *check
        s.testVhostRedirectTokenToCookie(c, "GET",
                "example.com/c="+arvadostest.FooCollection+"/foo",
                "?api_token="+arvadostest.ActiveToken,
-               "",
+               nil,
                "",
                http.StatusBadRequest,
                "cannot serve inline content at this URL (possible configuration error; see https://doc.arvados.org/install/install-keep-web.html#dns)\n",
@@ -454,7 +511,7 @@ func (s *IntegrationSuite) TestVhostRedirectQueryTokenRequestAttachment(c *check
        resp := s.testVhostRedirectTokenToCookie(c, "GET",
                arvadostest.FooCollection+".example.com/foo",
                "?disposition=attachment&api_token="+arvadostest.ActiveToken,
-               "",
+               nil,
                "",
                http.StatusOK,
                "foo",
@@ -467,7 +524,7 @@ func (s *IntegrationSuite) TestVhostRedirectQueryTokenSiteFS(c *check.C) {
        resp := s.testVhostRedirectTokenToCookie(c, "GET",
                "download.example.com/by_id/"+arvadostest.FooCollection+"/foo",
                "?api_token="+arvadostest.ActiveToken,
-               "",
+               nil,
                "",
                http.StatusOK,
                "foo",
@@ -480,7 +537,7 @@ func (s *IntegrationSuite) TestPastCollectionVersionFileAccess(c *check.C) {
        resp := s.testVhostRedirectTokenToCookie(c, "GET",
                "download.example.com/c="+arvadostest.WazVersion1Collection+"/waz",
                "?api_token="+arvadostest.ActiveToken,
-               "",
+               nil,
                "",
                http.StatusOK,
                "waz",
@@ -489,7 +546,7 @@ func (s *IntegrationSuite) TestPastCollectionVersionFileAccess(c *check.C) {
        resp = s.testVhostRedirectTokenToCookie(c, "GET",
                "download.example.com/by_id/"+arvadostest.WazVersion1Collection+"/waz",
                "?api_token="+arvadostest.ActiveToken,
-               "",
+               nil,
                "",
                http.StatusOK,
                "waz",
@@ -502,7 +559,7 @@ func (s *IntegrationSuite) TestVhostRedirectQueryTokenTrustAllContent(c *check.C
        s.testVhostRedirectTokenToCookie(c, "GET",
                "example.com/c="+arvadostest.FooCollection+"/foo",
                "?api_token="+arvadostest.ActiveToken,
-               "",
+               nil,
                "",
                http.StatusOK,
                "foo",
@@ -515,7 +572,7 @@ func (s *IntegrationSuite) TestVhostRedirectQueryTokenAttachmentOnlyHost(c *chec
        s.testVhostRedirectTokenToCookie(c, "GET",
                "example.com/c="+arvadostest.FooCollection+"/foo",
                "?api_token="+arvadostest.ActiveToken,
-               "",
+               nil,
                "",
                http.StatusBadRequest,
                "cannot serve inline content at this URL (possible configuration error; see https://doc.arvados.org/install/install-keep-web.html#dns)\n",
@@ -524,7 +581,7 @@ func (s *IntegrationSuite) TestVhostRedirectQueryTokenAttachmentOnlyHost(c *chec
        resp := s.testVhostRedirectTokenToCookie(c, "GET",
                "example.com:1234/c="+arvadostest.FooCollection+"/foo",
                "?api_token="+arvadostest.ActiveToken,
-               "",
+               nil,
                "",
                http.StatusOK,
                "foo",
@@ -536,7 +593,7 @@ func (s *IntegrationSuite) TestVhostRedirectPOSTFormTokenToCookie(c *check.C) {
        s.testVhostRedirectTokenToCookie(c, "POST",
                arvadostest.FooCollection+".example.com/foo",
                "",
-               "application/x-www-form-urlencoded",
+               http.Header{"Content-Type": {"application/x-www-form-urlencoded"}},
                url.Values{"api_token": {arvadostest.ActiveToken}}.Encode(),
                http.StatusOK,
                "foo",
@@ -547,7 +604,7 @@ func (s *IntegrationSuite) TestVhostRedirectPOSTFormTokenToCookie404(c *check.C)
        s.testVhostRedirectTokenToCookie(c, "POST",
                arvadostest.FooCollection+".example.com/foo",
                "",
-               "application/x-www-form-urlencoded",
+               http.Header{"Content-Type": {"application/x-www-form-urlencoded"}},
                url.Values{"api_token": {arvadostest.SpectatorToken}}.Encode(),
                http.StatusNotFound,
                notFoundMessage+"\n",
@@ -559,7 +616,7 @@ func (s *IntegrationSuite) TestAnonymousTokenOK(c *check.C) {
        s.testVhostRedirectTokenToCookie(c, "GET",
                "example.com/c="+arvadostest.HelloWorldCollection+"/Hello%20world.txt",
                "",
-               "",
+               nil,
                "",
                http.StatusOK,
                "Hello world\n",
@@ -571,7 +628,7 @@ func (s *IntegrationSuite) TestAnonymousTokenError(c *check.C) {
        s.testVhostRedirectTokenToCookie(c, "GET",
                "example.com/c="+arvadostest.HelloWorldCollection+"/Hello%20world.txt",
                "",
-               "",
+               nil,
                "",
                http.StatusNotFound,
                notFoundMessage+"\n",
@@ -711,14 +768,18 @@ func (s *IntegrationSuite) TestXHRNoRedirect(c *check.C) {
        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 {
+func (s *IntegrationSuite) testVhostRedirectTokenToCookie(c *check.C, method, hostPath, queryString string, reqHeader http.Header, reqBody string, expectStatus int, expectRespBody string) *httptest.ResponseRecorder {
+       if reqHeader == nil {
+               reqHeader = http.Header{}
+       }
        u, _ := url.Parse(`http://` + hostPath + queryString)
+       c.Logf("requesting %s", u)
        req := &http.Request{
                Method:     method,
                Host:       u.Host,
                URL:        u,
                RequestURI: u.RequestURI(),
-               Header:     http.Header{"Content-Type": {contentType}},
+               Header:     reqHeader,
                Body:       ioutil.NopCloser(strings.NewReader(reqBody)),
        }
 
@@ -733,15 +794,18 @@ func (s *IntegrationSuite) testVhostRedirectTokenToCookie(c *check.C, method, ho
                return resp
        }
        c.Check(resp.Body.String(), check.Matches, `.*href="http://`+regexp.QuoteMeta(html.EscapeString(hostPath))+`(\?[^"]*)?".*`)
+       c.Check(strings.Split(resp.Header().Get("Location"), "?")[0], check.Equals, "http://"+hostPath)
        cookies := (&http.Response{Header: resp.Header()}).Cookies()
 
-       u, _ = u.Parse(resp.Header().Get("Location"))
+       u, err := u.Parse(resp.Header().Get("Location"))
+       c.Assert(err, check.IsNil)
+       c.Logf("following redirect to %s", u)
        req = &http.Request{
                Method:     "GET",
                Host:       u.Host,
                URL:        u,
                RequestURI: u.RequestURI(),
-               Header:     http.Header{},
+               Header:     reqHeader,
        }
        for _, c := range cookies {
                req.AddCookie(c)
@@ -749,7 +813,10 @@ func (s *IntegrationSuite) testVhostRedirectTokenToCookie(c *check.C, method, ho
 
        resp = httptest.NewRecorder()
        s.handler.ServeHTTP(resp, req)
-       c.Check(resp.Header().Get("Location"), check.Equals, "")
+
+       if resp.Code != http.StatusSeeOther {
+               c.Check(resp.Header().Get("Location"), check.Equals, "")
+       }
        return resp
 }