Merge branch '21535-multi-wf-delete'
[arvados.git] / services / keep-web / handler_test.go
index 3b41d80cfc31f0eab7e0c86102b217ea93db344b..07c7016d3a8e485e8b1267d73fd0b547b14662bc 100644 (file)
@@ -18,6 +18,7 @@ import (
        "path/filepath"
        "regexp"
        "strings"
+       "sync"
        "time"
 
        "git.arvados.org/arvados.git/lib/config"
@@ -59,6 +60,7 @@ func (s *UnitSuite) SetUpTest(c *check.C) {
                        logger:   logger,
                        registry: prometheus.NewRegistry(),
                },
+               metrics: newMetrics(prometheus.NewRegistry()),
        }
 }
 
@@ -83,7 +85,7 @@ func (s *UnitSuite) TestCORSPreflight(c *check.C) {
        c.Check(resp.Body.String(), check.Equals, "")
        c.Check(resp.Header().Get("Access-Control-Allow-Origin"), check.Equals, "*")
        c.Check(resp.Header().Get("Access-Control-Allow-Methods"), check.Equals, "COPY, DELETE, GET, LOCK, MKCOL, MOVE, OPTIONS, POST, PROPFIND, PROPPATCH, PUT, RMCOL, UNLOCK")
-       c.Check(resp.Header().Get("Access-Control-Allow-Headers"), check.Equals, "Authorization, Content-Type, Range, Depth, Destination, If, Lock-Token, Overwrite, Timeout")
+       c.Check(resp.Header().Get("Access-Control-Allow-Headers"), check.Equals, "Authorization, Content-Type, Range, Depth, Destination, If, Lock-Token, Overwrite, Timeout, Cache-Control")
 
        // Check preflight for a disallowed request
        resp = httptest.NewRecorder()
@@ -93,7 +95,125 @@ func (s *UnitSuite) TestCORSPreflight(c *check.C) {
        c.Check(resp.Code, check.Equals, http.StatusMethodNotAllowed)
 }
 
+func (s *UnitSuite) TestWebdavPrefixAndSource(c *check.C) {
+       for _, trial := range []struct {
+               method   string
+               path     string
+               prefix   string
+               source   string
+               notFound bool
+               seeOther bool
+       }{
+               {
+                       method: "PROPFIND",
+                       path:   "/",
+               },
+               {
+                       method: "PROPFIND",
+                       path:   "/dir1",
+               },
+               {
+                       method: "PROPFIND",
+                       path:   "/dir1/",
+               },
+               {
+                       method: "PROPFIND",
+                       path:   "/dir1/foo",
+                       prefix: "/dir1",
+                       source: "/dir1",
+               },
+               {
+                       method: "PROPFIND",
+                       path:   "/prefix/dir1/foo",
+                       prefix: "/prefix/",
+                       source: "",
+               },
+               {
+                       method: "PROPFIND",
+                       path:   "/prefix/dir1/foo",
+                       prefix: "/prefix",
+                       source: "",
+               },
+               {
+                       method: "PROPFIND",
+                       path:   "/prefix/dir1/foo",
+                       prefix: "/prefix/",
+                       source: "/",
+               },
+               {
+                       method: "PROPFIND",
+                       path:   "/prefix/foo",
+                       prefix: "/prefix/",
+                       source: "/dir1/",
+               },
+               {
+                       method: "GET",
+                       path:   "/prefix/foo",
+                       prefix: "/prefix/",
+                       source: "/dir1/",
+               },
+               {
+                       method: "PROPFIND",
+                       path:   "/prefix/",
+                       prefix: "/prefix",
+                       source: "/dir1",
+               },
+               {
+                       method: "PROPFIND",
+                       path:   "/prefix",
+                       prefix: "/prefix",
+                       source: "/dir1/",
+               },
+               {
+                       method:   "GET",
+                       path:     "/prefix",
+                       prefix:   "/prefix",
+                       source:   "/dir1",
+                       seeOther: true,
+               },
+               {
+                       method:   "PROPFIND",
+                       path:     "/dir1/foo",
+                       prefix:   "",
+                       source:   "/dir1",
+                       notFound: true,
+               },
+       } {
+               c.Logf("trial %+v", trial)
+               u := mustParseURL("http://" + arvadostest.FooBarDirCollection + ".keep-web.example" + trial.path)
+               req := &http.Request{
+                       Method:     trial.method,
+                       Host:       u.Host,
+                       URL:        u,
+                       RequestURI: u.RequestURI(),
+                       Header: http.Header{
+                               "Authorization":   {"Bearer " + arvadostest.ActiveTokenV2},
+                               "X-Webdav-Prefix": {trial.prefix},
+                               "X-Webdav-Source": {trial.source},
+                       },
+                       Body: ioutil.NopCloser(bytes.NewReader(nil)),
+               }
+
+               resp := httptest.NewRecorder()
+               s.handler.ServeHTTP(resp, req)
+               if trial.notFound {
+                       c.Check(resp.Code, check.Equals, http.StatusNotFound)
+               } else if trial.method == "PROPFIND" {
+                       c.Check(resp.Code, check.Equals, http.StatusMultiStatus)
+                       c.Check(resp.Body.String(), check.Matches, `(?ms).*>\n?$`)
+               } else if trial.seeOther {
+                       c.Check(resp.Code, check.Equals, http.StatusSeeOther)
+               } else {
+                       c.Check(resp.Code, check.Equals, http.StatusOK)
+               }
+       }
+}
+
 func (s *UnitSuite) TestEmptyResponse(c *check.C) {
+       // Ensure we start with an empty cache
+       defer os.Setenv("HOME", os.Getenv("HOME"))
+       os.Setenv("HOME", c.MkDir())
+
        for _, trial := range []struct {
                dataExists    bool
                sendIMSHeader bool
@@ -213,9 +333,10 @@ func (s *IntegrationSuite) TestVhostViaAuthzHeaderOAuth2(c *check.C) {
        s.doVhostRequests(c, authzViaAuthzHeaderOAuth2)
 }
 func authzViaAuthzHeaderOAuth2(r *http.Request, tok string) int {
-       r.Header.Add("Authorization", "Bearer "+tok)
+       r.Header.Add("Authorization", "OAuth2 "+tok)
        return http.StatusUnauthorized
 }
+
 func (s *IntegrationSuite) TestVhostViaAuthzHeaderBearer(c *check.C) {
        s.doVhostRequests(c, authzViaAuthzHeaderBearer)
 }
@@ -235,6 +356,27 @@ func authzViaCookieValue(r *http.Request, tok string) int {
        return http.StatusUnauthorized
 }
 
+func (s *IntegrationSuite) TestVhostViaHTTPBasicAuth(c *check.C) {
+       s.doVhostRequests(c, authzViaHTTPBasicAuth)
+}
+func authzViaHTTPBasicAuth(r *http.Request, tok string) int {
+       r.AddCookie(&http.Cookie{
+               Name:  "arvados_api_token",
+               Value: auth.EncodeTokenCookie([]byte(tok)),
+       })
+       return http.StatusUnauthorized
+}
+
+func (s *IntegrationSuite) TestVhostViaHTTPBasicAuthWithExtraSpaceChars(c *check.C) {
+       s.doVhostRequests(c, func(r *http.Request, tok string) int {
+               r.AddCookie(&http.Cookie{
+                       Name:  "arvados_api_token",
+                       Value: auth.EncodeTokenCookie([]byte(" " + tok + "\n")),
+               })
+               return http.StatusUnauthorized
+       })
+}
+
 func (s *IntegrationSuite) TestVhostViaPath(c *check.C) {
        s.doVhostRequests(c, authzViaPath)
 }
@@ -682,6 +824,34 @@ func (s *IntegrationSuite) TestVhostRedirectQueryTokenAttachmentOnlyHost(c *chec
        c.Check(resp.Header().Get("Content-Disposition"), check.Equals, "attachment")
 }
 
+func (s *IntegrationSuite) TestVhostRedirectMultipleTokens(c *check.C) {
+       baseUrl := arvadostest.FooCollection + ".example.com/foo"
+       query := url.Values{}
+
+       // The intent of these tests is to check that requests are redirected
+       // correctly in the presence of multiple API tokens. The exact response
+       // codes and content are not closely considered: they're just how
+       // keep-web responded when we made the smallest possible fix. Changing
+       // those responses may be okay, but you should still test all these
+       // different cases and the associated redirect logic.
+       query["api_token"] = []string{arvadostest.ActiveToken, arvadostest.AnonymousToken}
+       s.testVhostRedirectTokenToCookie(c, "GET", baseUrl, "?"+query.Encode(), nil, "", http.StatusOK, "foo")
+       query["api_token"] = []string{arvadostest.ActiveToken, arvadostest.AnonymousToken, ""}
+       s.testVhostRedirectTokenToCookie(c, "GET", baseUrl, "?"+query.Encode(), nil, "", http.StatusOK, "foo")
+       query["api_token"] = []string{arvadostest.ActiveToken, "", arvadostest.AnonymousToken}
+       s.testVhostRedirectTokenToCookie(c, "GET", baseUrl, "?"+query.Encode(), nil, "", http.StatusOK, "foo")
+       query["api_token"] = []string{"", arvadostest.ActiveToken}
+       s.testVhostRedirectTokenToCookie(c, "GET", baseUrl, "?"+query.Encode(), nil, "", http.StatusOK, "foo")
+
+       expectContent := regexp.QuoteMeta(unauthorizedMessage + "\n")
+       query["api_token"] = []string{arvadostest.AnonymousToken, "invalidtoo"}
+       s.testVhostRedirectTokenToCookie(c, "GET", baseUrl, "?"+query.Encode(), nil, "", http.StatusUnauthorized, expectContent)
+       query["api_token"] = []string{arvadostest.AnonymousToken, ""}
+       s.testVhostRedirectTokenToCookie(c, "GET", baseUrl, "?"+query.Encode(), nil, "", http.StatusUnauthorized, expectContent)
+       query["api_token"] = []string{"", arvadostest.AnonymousToken}
+       s.testVhostRedirectTokenToCookie(c, "GET", baseUrl, "?"+query.Encode(), nil, "", http.StatusUnauthorized, expectContent)
+}
+
 func (s *IntegrationSuite) TestVhostRedirectPOSTFormTokenToCookie(c *check.C) {
        s.testVhostRedirectTokenToCookie(c, "POST",
                arvadostest.FooCollection+".example.com/foo",
@@ -884,20 +1054,36 @@ func (s *IntegrationSuite) testVhostRedirectTokenToCookie(c *check.C, method, ho
 
        s.handler.ServeHTTP(resp, req)
        if resp.Code != http.StatusSeeOther {
+               attachment, _ := regexp.MatchString(`^attachment(;|$)`, resp.Header().Get("Content-Disposition"))
+               // Since we're not redirecting, check that any api_token in the URL is
+               // handled safely.
+               // If there is no token in the URL, then we're good.
+               // Otherwise, if the response code is an error, the body is expected to
+               // be static content, and nothing that might maliciously introspect the
+               // URL. It's considered safe and allowed.
+               // Otherwise, if the response content has attachment disposition,
+               // that's considered safe for all the reasons explained in the
+               // safeAttachment comment in handler.go.
+               c.Check(!u.Query().Has("api_token") || resp.Code >= 400 || attachment, check.Equals, true)
                return resp
        }
+
+       loc, err := url.Parse(resp.Header().Get("Location"))
+       c.Assert(err, check.IsNil)
+       c.Check(loc.Scheme, check.Equals, u.Scheme)
+       c.Check(loc.Host, check.Equals, u.Host)
+       c.Check(loc.RawPath, check.Equals, u.RawPath)
+       // If the response was a redirect, it should never include an API token.
+       c.Check(loc.Query().Has("api_token"), check.Equals, false)
        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, 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(),
+               Host:       loc.Host,
+               URL:        loc,
+               RequestURI: loc.RequestURI(),
                Header:     reqHeader,
        }
        for _, c := range cookies {
@@ -924,6 +1110,17 @@ func (s *IntegrationSuite) TestDirectoryListingWithNoAnonymousToken(c *check.C)
 }
 
 func (s *IntegrationSuite) testDirectoryListing(c *check.C) {
+       // The "ownership cycle" test fixtures are reachable from the
+       // "filter group without filters" group, causing webdav's
+       // walkfs to recurse indefinitely. Avoid that by deleting one
+       // of the bogus fixtures.
+       arv := arvados.NewClientFromEnv()
+       err := arv.RequestAndDecode(nil, "DELETE", "arvados/v1/groups/zzzzz-j7d0g-cx2al9cqkmsf1hs", nil, nil)
+       if err != nil {
+               c.Assert(err, check.FitsTypeOf, &arvados.TransactionError{})
+               c.Check(err.(*arvados.TransactionError).StatusCode, check.Equals, 404)
+       }
+
        s.handler.Cluster.Services.WebDAVDownload.ExternalURL.Host = "download.example.com"
        authHeader := http.Header{
                "Authorization": {"OAuth2 " + arvadostest.ActiveToken},
@@ -1060,8 +1257,32 @@ func (s *IntegrationSuite) testDirectoryListing(c *check.C) {
                        expect:  []string{"waz"},
                        cutDirs: 2,
                },
+               {
+                       uri:     "download.example.com/users/active/This filter group/",
+                       header:  authHeader,
+                       expect:  []string{"A Subproject/"},
+                       cutDirs: 3,
+               },
+               {
+                       uri:     "download.example.com/users/active/This filter group/A Subproject",
+                       header:  authHeader,
+                       expect:  []string{"baz_file/"},
+                       cutDirs: 4,
+               },
+               {
+                       uri:     "download.example.com/by_id/" + arvadostest.AFilterGroupUUID,
+                       header:  authHeader,
+                       expect:  []string{"A Subproject/"},
+                       cutDirs: 2,
+               },
+               {
+                       uri:     "download.example.com/by_id/" + arvadostest.AFilterGroupUUID + "/A Subproject",
+                       header:  authHeader,
+                       expect:  []string{"baz_file/"},
+                       cutDirs: 3,
+               },
        } {
-               comment := check.Commentf("HTML: %q => %q", trial.uri, trial.expect)
+               comment := check.Commentf("HTML: %q redir %q => %q", trial.uri, trial.redirect, trial.expect)
                resp := httptest.NewRecorder()
                u := mustParseURL("//" + trial.uri)
                req := &http.Request{
@@ -1097,6 +1318,7 @@ func (s *IntegrationSuite) testDirectoryListing(c *check.C) {
                } else {
                        c.Check(resp.Code, check.Equals, http.StatusOK, comment)
                        for _, e := range trial.expect {
+                               e = strings.Replace(e, " ", "%20", -1)
                                c.Check(resp.Body.String(), check.Matches, `(?ms).*href="./`+e+`".*`, comment)
                        }
                        c.Check(resp.Body.String(), check.Matches, `(?ms).*--cut-dirs=`+fmt.Sprintf("%d", trial.cutDirs)+` .*`, comment)
@@ -1129,6 +1351,12 @@ func (s *IntegrationSuite) testDirectoryListing(c *check.C) {
                }
                resp = httptest.NewRecorder()
                s.handler.ServeHTTP(resp, req)
+               // This check avoids logging a big XML document in the
+               // event webdav throws a 500 error after sending
+               // headers for a 207.
+               if !c.Check(strings.HasSuffix(resp.Body.String(), "Internal Server Error"), check.Equals, false) {
+                       continue
+               }
                if trial.expect == nil {
                        c.Check(resp.Code, check.Equals, http.StatusUnauthorized, comment)
                } else {
@@ -1139,6 +1367,7 @@ func (s *IntegrationSuite) testDirectoryListing(c *check.C) {
                                } else {
                                        e = filepath.Join(u.Path, e)
                                }
+                               e = strings.Replace(e, " ", "%20", -1)
                                c.Check(resp.Body.String(), check.Matches, `(?ms).*<D:href>`+e+`</D:href>.*`, comment)
                        }
                }
@@ -1245,20 +1474,14 @@ func (s *IntegrationSuite) TestFileContentType(c *check.C) {
        }
 }
 
-func (s *IntegrationSuite) TestKeepClientBlockCache(c *check.C) {
-       s.handler.Cluster.Collections.WebDAVCache.MaxBlockEntries = 42
-       c.Check(keepclient.DefaultBlockCache.MaxBlocks, check.Not(check.Equals), 42)
-       u := mustParseURL("http://keep-web.example/c=" + arvadostest.FooCollection + "/t=" + arvadostest.ActiveToken + "/foo")
-       req := &http.Request{
-               Method:     "GET",
-               Host:       u.Host,
-               URL:        u,
-               RequestURI: u.RequestURI(),
-       }
+func (s *IntegrationSuite) TestCacheSize(c *check.C) {
+       req, err := http.NewRequest("GET", "http://"+arvadostest.FooCollection+".example.com/foo", nil)
+       req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveTokenV2)
+       c.Assert(err, check.IsNil)
        resp := httptest.NewRecorder()
        s.handler.ServeHTTP(resp, req)
-       c.Check(resp.Code, check.Equals, http.StatusOK)
-       c.Check(keepclient.DefaultBlockCache.MaxBlocks, check.Equals, 42)
+       c.Assert(resp.Code, check.Equals, http.StatusOK)
+       c.Check(s.handler.Cache.sessions[arvadostest.ActiveTokenV2].client.DiskCacheSize.Percent(), check.Equals, int64(10))
 }
 
 // Writing to a collection shouldn't affect its entry in the
@@ -1510,3 +1733,72 @@ func (s *IntegrationSuite) TestUploadLoggingPermission(c *check.C) {
                }
        }
 }
+
+func (s *IntegrationSuite) TestConcurrentWrites(c *check.C) {
+       s.handler.Cluster.Collections.WebDAVCache.TTL = arvados.Duration(time.Second * 2)
+       lockTidyInterval = time.Second
+       client := arvados.NewClientFromEnv()
+       client.AuthToken = arvadostest.ActiveTokenV2
+       // Start small, and increase concurrency (2^2, 4^2, ...)
+       // only until hitting failure. Avoids unnecessarily long
+       // failure reports.
+       for n := 2; n < 16 && !c.Failed(); n = n * 2 {
+               c.Logf("%s: n=%d", c.TestName(), n)
+
+               var coll arvados.Collection
+               err := client.RequestAndDecode(&coll, "POST", "arvados/v1/collections", nil, nil)
+               c.Assert(err, check.IsNil)
+               defer client.RequestAndDecode(&coll, "DELETE", "arvados/v1/collections/"+coll.UUID, nil, nil)
+
+               var wg sync.WaitGroup
+               for i := 0; i < n && !c.Failed(); i++ {
+                       i := i
+                       wg.Add(1)
+                       go func() {
+                               defer wg.Done()
+                               u := mustParseURL(fmt.Sprintf("http://%s.collections.example.com/i=%d", coll.UUID, i))
+                               resp := httptest.NewRecorder()
+                               req, err := http.NewRequest("MKCOL", u.String(), nil)
+                               c.Assert(err, check.IsNil)
+                               req.Header.Set("Authorization", "Bearer "+client.AuthToken)
+                               s.handler.ServeHTTP(resp, req)
+                               c.Assert(resp.Code, check.Equals, http.StatusCreated)
+                               for j := 0; j < n && !c.Failed(); j++ {
+                                       j := j
+                                       wg.Add(1)
+                                       go func() {
+                                               defer wg.Done()
+                                               content := fmt.Sprintf("i=%d/j=%d", i, j)
+                                               u := mustParseURL("http://" + coll.UUID + ".collections.example.com/" + content)
+
+                                               resp := httptest.NewRecorder()
+                                               req, err := http.NewRequest("PUT", u.String(), strings.NewReader(content))
+                                               c.Assert(err, check.IsNil)
+                                               req.Header.Set("Authorization", "Bearer "+client.AuthToken)
+                                               s.handler.ServeHTTP(resp, req)
+                                               c.Check(resp.Code, check.Equals, http.StatusCreated)
+
+                                               time.Sleep(time.Second)
+                                               resp = httptest.NewRecorder()
+                                               req, err = http.NewRequest("GET", u.String(), nil)
+                                               c.Assert(err, check.IsNil)
+                                               req.Header.Set("Authorization", "Bearer "+client.AuthToken)
+                                               s.handler.ServeHTTP(resp, req)
+                                               c.Check(resp.Code, check.Equals, http.StatusOK)
+                                               c.Check(resp.Body.String(), check.Equals, content)
+                                       }()
+                               }
+                       }()
+               }
+               wg.Wait()
+               for i := 0; i < n; i++ {
+                       u := mustParseURL(fmt.Sprintf("http://%s.collections.example.com/i=%d", coll.UUID, i))
+                       resp := httptest.NewRecorder()
+                       req, err := http.NewRequest("PROPFIND", u.String(), &bytes.Buffer{})
+                       c.Assert(err, check.IsNil)
+                       req.Header.Set("Authorization", "Bearer "+client.AuthToken)
+                       s.handler.ServeHTTP(resp, req)
+                       c.Assert(resp.Code, check.Equals, http.StatusMultiStatus)
+               }
+       }
+}