X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/985e3c8b29f684dace905bf787d4b870a4cdfe63..894e1a3d70e9ec1b3e8619d1822410d665fabab4:/services/keep-web/handler_test.go diff --git a/services/keep-web/handler_test.go b/services/keep-web/handler_test.go index fac75214d6..5e81f3a01e 100644 --- a/services/keep-web/handler_test.go +++ b/services/keep-web/handler_test.go @@ -18,6 +18,7 @@ import ( "path/filepath" "regexp" "strings" + "sync" "time" "git.arvados.org/arvados.git/lib/config" @@ -83,7 +84,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,6 +94,120 @@ 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) { for _, trial := range []struct { dataExists bool @@ -551,6 +666,41 @@ func (s *IntegrationSuite) TestVhostRedirectWithNoCache(c *check.C) { c.Check(u.Query().Get("redirectToDownload"), check.Equals, "") } +func (s *IntegrationSuite) TestNoTokenWorkbench2LoginFlow(c *check.C) { + for _, trial := range []struct { + anonToken bool + cacheControl string + }{ + {}, + {cacheControl: "no-cache"}, + {anonToken: true}, + {anonToken: true, cacheControl: "no-cache"}, + } { + c.Logf("trial: %+v", trial) + + if trial.anonToken { + s.handler.Cluster.Users.AnonymousUserToken = arvadostest.AnonymousToken + } else { + s.handler.Cluster.Users.AnonymousUserToken = "" + } + req, err := http.NewRequest("GET", "http://"+arvadostest.FooCollection+".example.com/foo", nil) + c.Assert(err, check.IsNil) + req.Header.Set("Sec-Fetch-Mode", "navigate") + if trial.cacheControl != "" { + req.Header.Set("Cache-Control", trial.cacheControl) + } + resp := httptest.NewRecorder() + s.handler.ServeHTTP(resp, req) + c.Check(resp.Code, check.Equals, http.StatusSeeOther) + u, err := url.Parse(resp.Header().Get("Location")) + c.Assert(err, check.IsNil) + c.Logf("redirected to %q", 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, "") + } +} + func (s *IntegrationSuite) TestVhostRedirectQueryTokenSingleOriginError(c *check.C) { s.testVhostRedirectTokenToCookie(c, "GET", "example.com/c="+arvadostest.FooCollection+"/foo", @@ -647,6 +797,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", @@ -849,20 +1027,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 { @@ -1475,3 +1669,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) + } + } +}