17464: Clean up tests

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz@curii.com>

diff --git a/services/keep-web/handler.go b/services/keep-web/handler.go
index 43ce57904c5c31dde7d855482afa87341a4fb178..6d0b7669e3921a43f40ae6e2da370662d2ddc64a 100644 (file)
--- 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</A>`)
 }
 
-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 726c6220e6caabd905e0af1bb7ec97bb6aeeecf2..65dfb4312f7ff33ba64403967721c91255d15c02 100644 (file)
--- 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 357c42ae6caee6e84f15c2c20d0f2ac754caa911..e6262374d640f9ed526ef38c816fa785bce1d3d5 100644 (file)
--- a/services/keep-web/s3.go
+++ b/services/keep-web/s3.go
@@ -406,11 +406,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
@@ -497,11 +497,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 {