From: Peter Amstutz Date: Thu, 17 Jun 2021 18:48:33 +0000 (-0400) Subject: 17464: Clean up tests X-Git-Tag: 2.2.1~45 X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/2a690ac309678e952a78609039176224a99d3c06 17464: Clean up tests Arvados-DCO-1.1-Signed-off-by: Peter Amstutz --- diff --git a/services/keep-web/handler.go b/services/keep-web/handler.go index 43ce57904c..6d0b7669e3 100644 --- a/services/keep-web/handler.go +++ b/services/keep-web/handler.go @@ -487,11 +487,11 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) { // Check configured permission _, sess, err := h.Config.Cache.GetSession(arv.ApiToken) tokenUser, err = h.Config.Cache.GetTokenUser(arv.ApiToken) - if !h.UserPermittedToUploadOrDownload(r.Method, tokenUser) { + if !h.userPermittedToUploadOrDownload(r.Method, tokenUser) { http.Error(w, "Not permitted", http.StatusForbidden) return } - h.LogUploadOrDownload(r, sess.arvadosclient, nil, strings.Join(targetPath, "/"), collection, tokenUser) + h.logUploadOrDownload(r, sess.arvadosclient, nil, strings.Join(targetPath, "/"), collection, tokenUser) if webdavMethod[r.Method] { if writeMethod[r.Method] { @@ -619,11 +619,11 @@ func (h *handler) serveSiteFS(w http.ResponseWriter, r *http.Request, tokens []s } tokenUser, err := h.Config.Cache.GetTokenUser(tokens[0]) - if !h.UserPermittedToUploadOrDownload(r.Method, tokenUser) { + if !h.userPermittedToUploadOrDownload(r.Method, tokenUser) { http.Error(w, "Not permitted", http.StatusForbidden) return } - h.LogUploadOrDownload(r, sess.arvadosclient, fs, r.URL.Path, nil, tokenUser) + h.logUploadOrDownload(r, sess.arvadosclient, fs, r.URL.Path, nil, tokenUser) if r.Method == "GET" { _, basename := filepath.Split(r.URL.Path) @@ -856,18 +856,18 @@ func (h *handler) seeOtherWithCookie(w http.ResponseWriter, r *http.Request, loc io.WriteString(w, `">Continue`) } -func (h *handler) UserPermittedToUploadOrDownload(method string, tokenUser *arvados.User) bool { +func (h *handler) userPermittedToUploadOrDownload(method string, tokenUser *arvados.User) bool { if tokenUser == nil { return false } var permitDownload bool var permitUpload bool if tokenUser.IsAdmin { - permitUpload = h.Config.cluster.Collections.KeepWebPermission.Admin.Upload - permitDownload = h.Config.cluster.Collections.KeepWebPermission.Admin.Download + permitUpload = h.Config.cluster.Collections.WebDAVPermission.Admin.Upload + permitDownload = h.Config.cluster.Collections.WebDAVPermission.Admin.Download } else { - permitUpload = h.Config.cluster.Collections.KeepWebPermission.User.Upload - permitDownload = h.Config.cluster.Collections.KeepWebPermission.User.Download + permitUpload = h.Config.cluster.Collections.WebDAVPermission.User.Upload + permitDownload = h.Config.cluster.Collections.WebDAVPermission.User.Download } if (method == "PUT" || method == "POST") && !permitUpload { // Disallow operations that upload new files. @@ -882,7 +882,7 @@ func (h *handler) UserPermittedToUploadOrDownload(method string, tokenUser *arva return true } -func (h *handler) LogUploadOrDownload( +func (h *handler) logUploadOrDownload( r *http.Request, client *arvadosclient.ArvadosClient, fs arvados.CustomFileSystem, @@ -898,7 +898,7 @@ func (h *handler) LogUploadOrDownload( WithField("user_full_name", user.FullName) } if collection == nil && fs != nil { - collection, filepath = h.DetermineCollection(fs, filepath) + collection, filepath = h.determineCollection(fs, filepath) } if collection != nil { log = log.WithField("collection_uuid", collection.UUID). @@ -908,46 +908,63 @@ func (h *handler) LogUploadOrDownload( } if r.Method == "PUT" || r.Method == "POST" { log.Info("File upload") - go func() { - lr := arvadosclient.Dict{"log": arvadosclient.Dict{ - "object_uuid": user.UUID, - "event_type": "file_upload", - "properties": props}} - client.Create("logs", lr, nil) - }() + if h.Config.cluster.Collections.WebDAVLogEvents { + go func() { + lr := arvadosclient.Dict{"log": arvadosclient.Dict{ + "object_uuid": user.UUID, + "event_type": "file_upload", + "properties": props}} + err := client.Create("logs", lr, nil) + if err != nil { + log.WithError(err).Error("Failed to create upload log event on API server") + } + }() + } } else if r.Method == "GET" { if collection != nil && collection.PortableDataHash != "" { log = log.WithField("portable_data_hash", collection.PortableDataHash) props["portable_data_hash"] = collection.PortableDataHash } log.Info("File download") - go func() { - lr := arvadosclient.Dict{"log": arvadosclient.Dict{ - "object_uuid": user.UUID, - "event_type": "file_download", - "properties": props}} - client.Create("logs", lr, nil) - }() + if h.Config.cluster.Collections.WebDAVLogEvents { + go func() { + lr := arvadosclient.Dict{"log": arvadosclient.Dict{ + "object_uuid": user.UUID, + "event_type": "file_download", + "properties": props}} + err := client.Create("logs", lr, nil) + if err != nil { + log.WithError(err).Error("Failed to create download log event on API server") + } + }() + } } } -func (h *handler) DetermineCollection(fs arvados.CustomFileSystem, path string) (*arvados.Collection, string) { +func (h *handler) determineCollection(fs arvados.CustomFileSystem, path string) (*arvados.Collection, string) { segments := strings.Split(path, "/") var i int - for i = len(segments) - 1; i >= 0; i-- { + for i = 0; i < len(segments); i++ { dir := append([]string{}, segments[0:i]...) dir = append(dir, ".arvados#collection") f, err := fs.OpenFile(strings.Join(dir, "/"), os.O_RDONLY, 0) - if err == nil { - decoder := json.NewDecoder(f) - var collection arvados.Collection - err = decoder.Decode(&collection) - if err != nil { + if f != nil { + defer f.Close() + } + if err != nil { + if !os.IsNotExist(err) { return nil, "" } - return &collection, strings.Join(segments[i:], "/") + continue + } + // err is nil so we found it. + decoder := json.NewDecoder(f) + var collection arvados.Collection + err = decoder.Decode(&collection) + if err != nil { + return nil, "" } - f.Close() + return &collection, strings.Join(segments[i:], "/") } return nil, "" } diff --git a/services/keep-web/handler_test.go b/services/keep-web/handler_test.go index 726c6220e6..65dfb4312f 100644 --- a/services/keep-web/handler_test.go +++ b/services/keep-web/handler_test.go @@ -1246,8 +1246,8 @@ func (s *IntegrationSuite) TestDownloadLoggingPermission(c *check.C) { for _, adminperm := range []bool{true, false} { for _, userperm := range []bool{true, false} { - config.cluster.Collections.KeepWebPermission.Admin.Download = adminperm - config.cluster.Collections.KeepWebPermission.User.Download = userperm + config.cluster.Collections.WebDAVPermission.Admin.Download = adminperm + config.cluster.Collections.WebDAVPermission.User.Download = userperm // Test admin permission req := &http.Request{ @@ -1279,20 +1279,32 @@ func (s *IntegrationSuite) TestDownloadLoggingPermission(c *check.C) { } func (s *IntegrationSuite) TestUploadLoggingPermission(c *check.C) { - defer func() { - client := s.testServer.Config.Client - client.AuthToken = arvadostest.AdminToken - client.RequestAndDecode(nil, "POST", "database/reset", nil, nil) - }() - config := newConfig(s.ArvConfig) h := handler{Config: config} - u := mustParseURL("http://" + arvadostest.FooCollection + ".keep-web.example/bar") for _, adminperm := range []bool{true, false} { for _, userperm := range []bool{true, false} { - config.cluster.Collections.KeepWebPermission.Admin.Upload = adminperm - config.cluster.Collections.KeepWebPermission.User.Upload = userperm + + arv := s.testServer.Config.Client + arv.AuthToken = arvadostest.ActiveToken + + var coll arvados.Collection + err := arv.RequestAndDecode(&coll, + "POST", + "/arvados/v1/collections", + nil, + map[string]interface{}{ + "ensure_unique_name": true, + "collection": map[string]interface{}{ + "name": "test collection", + }, + }) + c.Assert(err, check.Equals, nil) + + u := mustParseURL("http://" + coll.UUID + ".keep-web.example/bar") + + config.cluster.Collections.WebDAVPermission.Admin.Upload = adminperm + config.cluster.Collections.WebDAVPermission.User.Upload = userperm // Test admin permission req := &http.Request{ @@ -1306,7 +1318,7 @@ func (s *IntegrationSuite) TestUploadLoggingPermission(c *check.C) { Body: io.NopCloser(bytes.NewReader([]byte("bar"))), } s.checkUploadDownloadRequest(c, &h, req, http.StatusCreated, "upload", adminperm, - arvadostest.AdminUserUUID, arvadostest.FooCollection, "bar") + arvadostest.AdminUserUUID, coll.UUID, "bar") // Test user permission req = &http.Request{ @@ -1320,7 +1332,7 @@ func (s *IntegrationSuite) TestUploadLoggingPermission(c *check.C) { Body: io.NopCloser(bytes.NewReader([]byte("bar"))), } s.checkUploadDownloadRequest(c, &h, req, http.StatusCreated, "upload", userperm, - arvadostest.ActiveUserUUID, arvadostest.FooCollection, "bar") + arvadostest.ActiveUserUUID, coll.UUID, "bar") } } } diff --git a/services/keep-web/s3.go b/services/keep-web/s3.go index 447c37cbc8..df70f6f8aa 100644 --- a/services/keep-web/s3.go +++ b/services/keep-web/s3.go @@ -382,11 +382,11 @@ func (h *handler) serveS3(w http.ResponseWriter, r *http.Request) bool { } tokenUser, err := h.Config.Cache.GetTokenUser(token) - if !h.UserPermittedToUploadOrDownload(r.Method, tokenUser) { + if !h.userPermittedToUploadOrDownload(r.Method, tokenUser) { http.Error(w, "Not permitted", http.StatusForbidden) return true } - h.LogUploadOrDownload(r, arvclient, fs, fspath, nil, tokenUser) + h.logUploadOrDownload(r, arvclient, fs, fspath, nil, tokenUser) // shallow copy r, and change URL path r := *r @@ -473,11 +473,11 @@ func (h *handler) serveS3(w http.ResponseWriter, r *http.Request) bool { defer f.Close() tokenUser, err := h.Config.Cache.GetTokenUser(token) - if !h.UserPermittedToUploadOrDownload(r.Method, tokenUser) { + if !h.userPermittedToUploadOrDownload(r.Method, tokenUser) { http.Error(w, "Not permitted", http.StatusForbidden) return true } - h.LogUploadOrDownload(r, arvclient, fs, fspath, nil, tokenUser) + h.logUploadOrDownload(r, arvclient, fs, fspath, nil, tokenUser) _, err = io.Copy(f, r.Body) if err != nil {