[ARVADOS] updated: 2.1.0-964-g10afc7ae2
Git user
git at public.arvados.org
Fri Jun 18 15:35:18 UTC 2021
Summary of changes:
apps/workbench/Gemfile.lock | 91 ++-
cmd/arvados-client/cmd.go | 2 +
doc/admin/upgrading.html.textile.liquid | 6 +-
.../install-shell-server.html.textile.liquid | 1 -
go.mod | 8 +-
go.sum | 21 +
lib/cmd/cmd.go | 8 +
lib/config/config.default.yml | 59 +-
lib/config/generated_config.go | 59 +-
lib/config/load.go | 32 +-
lib/config/load_test.go | 32 +-
lib/costanalyzer/cmd.go | 30 +-
lib/costanalyzer/costanalyzer.go | 200 +++++--
lib/costanalyzer/costanalyzer_test.go | 36 ++
lib/deduplicationreport/command.go | 10 +-
lib/diagnostics/cmd.go | 612 +++++++++++++++++++++
sdk/go/arvados/fs_base.go | 19 +
sdk/go/arvados/fs_collection.go | 11 +-
sdk/go/arvados/fs_site.go | 6 +
sdk/go/arvados/fs_site_test.go | 9 +
sdk/go/arvadostest/fixtures.go | 2 +
sdk/python/arvados/arvfile.py | 18 +-
sdk/python/arvados/collection.py | 33 +-
sdk/python/arvados/commands/put.py | 32 +-
sdk/python/arvados/keep.py | 123 ++++-
sdk/python/tests/test_arv_put.py | 31 +-
sdk/python/tests/test_arvfile.py | 14 +-
sdk/python/tests/test_collections.py | 43 +-
sdk/python/tests/test_keep_client.py | 100 +++-
services/api/Gemfile.lock | 91 ++-
services/api/app/models/container_request.rb | 14 +
services/api/test/fixtures/container_requests.yml | 30 +-
services/api/test/fixtures/containers.yml | 24 +-
services/api/test/unit/container_request_test.rb | 25 +
services/keep-web/handler_test.go | 4 +
services/keep-web/s3.go | 34 +-
services/keep-web/s3_test.go | 22 +-
tools/compute-images/scripts/base.sh | 4 +
tools/salt-install/provision.sh | 2 +-
39 files changed, 1537 insertions(+), 361 deletions(-)
create mode 100644 lib/diagnostics/cmd.go
discards eb9fa033e0d19a2459041f296e3286639ad4d1ff (commit)
discards d7fb2ac79d28575f074626b92fb91d7b9fdc0ca2 (commit)
discards 0184f9ea54273a7ad60aedeb7ddf823db0b9f83d (commit)
discards 32b7adff99dd2308c37cf00f4d83f55a216e8a4a (commit)
discards e4dea1aaa8afad47dc805095009aca44a630b921 (commit)
discards 699ddfc7ed0a8ea10def11df03e38d5a169f77d2 (commit)
discards 4b7a059db60e8abf361b9394542dd5eb2d05c79a (commit)
discards 0da30aa9a699faa5e735c3aca2f2d6cd96429944 (commit)
discards 8697aedb4507a98ca5c459321271c2c004b38b5f (commit)
discards 0698f0ee6f2748016688b81e7bd53c8efa200648 (commit)
discards fb343cbbf8157315ec1a34a547beb0f2ddd5314c (commit)
discards 9860df8c037d1230d543a06043d7af3c85ae2d27 (commit)
discards fa956f9afdf53aafaa5b8fab1bb923af03d6120c (commit)
discards d3796adf4fca639b6741e1e0ddbe049a32b5e157 (commit)
discards 3237f96388a0c0ca2ec299265c6e51eab55a2d2d (commit)
via 10afc7ae2f95aad09937465702be7c44d07920d4 (commit)
via 903e0d1c2e209f4d62e0d83ce3a062b823a71da3 (commit)
via 572e375e1c950f35d7e159bac031de5205354590 (commit)
via 046d4591c694537dc1a415744413986785fc6b9c (commit)
via 154ba68b8f66e50d9ddb53f3a5f19890588f42f4 (commit)
via a271aaf8e345f8035a8818cee15868bd1c38ceb0 (commit)
via e093e9ea2e7c204c5c58de006d8b316c9013b6dd (commit)
via 7ab068369174b2a4de6251202a021f20ca1f36f3 (commit)
via 59107add9b9d01d98cca49933c90f4e320d47fef (commit)
via 23f6c01aaf4a3f501e908a689f128e77fc9f23aa (commit)
via 3a8485be4eeb09f082338bc3dabe11a3702e538c (commit)
via ce4ece370e62774f1e3ef797c714529603943f41 (commit)
via 834ba51d22fa93297e66c60c3eb51cc1cf05fccc (commit)
via 69b64f2fe8ccd2b92d1aa55b9fb39b03a342468e (commit)
via de8448b3b546eb1eee8a45261954028e3ea22252 (commit)
via e506fa36be9b9b2cb67abb8cd964358394a3a4fe (commit)
via e627df2797dae0d6fa95da61f1a58bb9fafe8240 (commit)
via 9f55b356fb37e06d2aef1c1c5acb8a9878dfe85b (commit)
via 07b765184292a503111416ef0df168282350ce2e (commit)
via e45ba3bbf5aa851bdf8de7612eb6b587b615ba21 (commit)
via b2f695cf793f6c9b47607d93f5ed1af301f7f010 (commit)
via 9f06f5b2e5f5549c61458db7f401fc2e2dc7dd0b (commit)
via 2d3723c301085ebfcb1f3a8940451edc15f10f93 (commit)
via df90bb0a37a7e6fc3c18afd3e000a71fb7cc11e2 (commit)
via d86405e79b04df920e2b196e521959f33649183c (commit)
via 43e30e4be9104cc4e5968756339dbad957d4d556 (commit)
via 4b5ec8651ba83f2a79fe40708021e03c86275093 (commit)
via 84ae3e67dc2caa9ba040d38ec77a50367e172cae (commit)
via 96dc323b08e952ea2842d82871a7fe7490e0a501 (commit)
via ec8f96814a7bc8da38309c2e9ed1e5a0e14dcdad (commit)
via 4e65a41bea4b892ef7232bfe6b9b20ca35380368 (commit)
via 056b3d2368b151a626fbf79025d9989a4d29a018 (commit)
via 23f0fc06dbb6d7e82d820a8c65997f32c760f34e (commit)
via 523d1c2a9963edc25becf7958e024992ed8a6e66 (commit)
via 38218a359786e327bbd4f3379be19e25a5a005f9 (commit)
via ea1ade46c416421aabaa585d1df774e042967839 (commit)
via b7afa91b549fe3e49e11341196d8d7ab02ba78be (commit)
via bf963a4027160e35b7f526328f2c61f3aac78121 (commit)
via 4528ae5bcdfa43cd31a0d0419aa5c7caea1f017a (commit)
via f9ee759496885b89d034888227cb69955080a2a4 (commit)
via 59b4ca616c7d0c43da4481c77b4479ee59b94697 (commit)
via 75a18b03319f6b9b795d7fe46403c04fe795956f (commit)
via 08c8b9cc496627bc3fd3d87ae333fadce4797eaa (commit)
via 3975b33706d85857e8d7a8ec2677edbb35984a14 (commit)
via 93cf55af0be9ef96c79d17323d82ba85eb1b9968 (commit)
via 56d721569988522e5cca008a303b19d008b937dd (commit)
via dd7268b61d18f37036f40479c67b4b9f446eb2a0 (commit)
via 6c517a23f8af60ecbb87099be26f8df7a7652f4f (commit)
via 6560ade0a1fca34182814e0881b8b543216ad328 (commit)
via a98105b60cb4bd19ed0723bc14b7a5e4514e7a83 (commit)
via 5b3f3eee0352885c8daaca67f9d42f4480c761dd (commit)
via 751cdf33d5519c393323baa15233ca55b7c7a9a1 (commit)
via 2c843ad3115a94385815dc6925ecee219e3e1a93 (commit)
via 0b0b4c7b23e96a6efb3cfd88b0ba7224158e9544 (commit)
via cd6647b38be0ca224f98fe9e38ec315947e0f7f2 (commit)
via af9a350dcaa93e8d748aaa7e14721e96da8f212e (commit)
via 34d1e41a3e1fcf3abb2d85845a87b25014132959 (commit)
via 8858d25844dc2591e7465466de540c8a62ee4945 (commit)
via 41e0df276eaaa548e692e44cd8f3c27c4692375e (commit)
via 0e26c53b179763f279a9f6014fec0ac96c6dbeab (commit)
via 8227b8a1be943fbbf3adb23a3d549dec28efbbba (commit)
via 57a26e595dc47865da5d929271e269facd14d4af (commit)
via 205aecd0875343feaec88e7f0e78dd6e9db1fd9c (commit)
via e2dab6b34eba7f300ffebd217dd2344223b12c50 (commit)
via eba64f9495cfcf65203eee41e90765cf5b6732d6 (commit)
via fc6c587d40d66273c7520b934480cceaacec7a70 (commit)
via b5178b3f99de705e2afcdc91f763f779481131cb (commit)
via b4808aa380097befefde114a0a87f41270c862bb (commit)
via 5ab34436e2c47cebedd81fe6d38bdcbc40d6c373 (commit)
via 744bbbafa5dcbba814391eaedfa9489c3614b644 (commit)
via aeb5185342b751b6bbbf1e17024d6f17417ffaf2 (commit)
via e9ccdda4ebf616ece6a04e059f20e38414df3e5b (commit)
via a6419676c073a863232c4656f0602b2d038ec3cd (commit)
via 864baf0627216bb82475ed0323b4107f47cd7fd5 (commit)
via 05607eae460bab7efc61dd33d1e88b31139a97c7 (commit)
via 4c8b0d8f326cfe65ad98d948f62f7478db099afa (commit)
via 73deb6bf9837bad9150fa9268d6f1c76da866a5c (commit)
This update added new revisions after undoing existing revisions. That is
to say, the old revision is not a strict subset of the new revision. This
situation occurs when you --force push a change and generate a repository
containing something like this:
* -- * -- B -- O -- O -- O (eb9fa033e0d19a2459041f296e3286639ad4d1ff)
\
N -- N -- N (10afc7ae2f95aad09937465702be7c44d07920d4)
When this happens we assume that you've already had alert emails for all
of the O revisions, and so we here report only the revisions in the N
branch from the common base, B.
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit 10afc7ae2f95aad09937465702be7c44d07920d4
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Thu Jun 17 16:47:33 2021 -0400
17464: Don't deny or log GET on directories.
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/doc/admin/restricting-upload-download.html.textile.liquid b/doc/admin/restricting-upload-download.html.textile.liquid
index 45bcd8a07..6983e413f 100644
--- a/doc/admin/restricting-upload-download.html.textile.liquid
+++ b/doc/admin/restricting-upload-download.html.textile.liquid
@@ -39,7 +39,7 @@ Permitting @WebDAV@ makes it possible to use WebDAV, S3 API, download from Workb
When a user attempts to upload or download from a service without permission, they will receive a @403 Forbidden@ response. This only applies to file content.
-Denying download permission does not deny access to access to XML file listings with PROPFIND. As a side effect it does deny auto-generated HTML documents that have file listings.
+Denying download permission does not deny access to access to XML file listings with PROPFIND, or auto-generated HTML documents containing file listings.
Denying upload permission does not deny other operations that modify collections without directly accessing file content, such as MOVE and COPY.
diff --git a/services/keep-web/handler.go b/services/keep-web/handler.go
index 6d0b7669e..6f6ff542b 100644
--- a/services/keep-web/handler.go
+++ b/services/keep-web/handler.go
@@ -487,13 +487,14 @@ 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) {
- http.Error(w, "Not permitted", http.StatusForbidden)
- return
- }
- h.logUploadOrDownload(r, sess.arvadosclient, nil, strings.Join(targetPath, "/"), collection, tokenUser)
if webdavMethod[r.Method] {
+ 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)
+
if writeMethod[r.Method] {
// Save the collection only if/when all
// webdav->filesystem operations succeed --
@@ -548,6 +549,12 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
} else if stat.IsDir() {
h.serveDirectory(w, r, collection.Name, fs, openPath, true)
} else {
+ 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)
+
http.ServeContent(w, r, basename, stat.ModTime(), f)
if wrote := int64(w.WroteBodyBytes()); wrote != stat.Size() && w.WroteStatus() == http.StatusOK {
// If we wrote fewer bytes than expected, it's
commit 903e0d1c2e209f4d62e0d83ce3a062b823a71da3
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Thu Jun 17 15:49:04 2021 -0400
17464: Add tests for paths by /users/ and by PDH
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/sdk/go/arvadostest/fixtures.go b/sdk/go/arvadostest/fixtures.go
index 0c05441a8..4b7ad6dd5 100644
--- a/sdk/go/arvadostest/fixtures.go
+++ b/sdk/go/arvadostest/fixtures.go
@@ -31,6 +31,8 @@ const (
UserAgreementPDH = "b519d9cb706a29fc7ea24dbea2f05851+93"
HelloWorldPdh = "55713e6a34081eb03609e7ad5fcad129+62"
+ MultilevelCollection1 = "zzzzz-4zz18-pyw8yp9g3pr7irn"
+
AProjectUUID = "zzzzz-j7d0g-v955i6s2oi1cbso"
ASubprojectUUID = "zzzzz-j7d0g-axqo7eu9pwvna1x"
diff --git a/services/keep-web/handler_test.go b/services/keep-web/handler_test.go
index 65dfb4312..e883e806c 100644
--- a/services/keep-web/handler_test.go
+++ b/services/keep-web/handler_test.go
@@ -1216,7 +1216,8 @@ func (s *IntegrationSuite) checkUploadDownloadRequest(c *check.C, h *handler, re
c.Check(logbuf.String(), check.Matches, `(?ms).*msg="File `+direction+`".*`)
c.Check(logbuf.String(), check.Not(check.Matches), `(?ms).*level=error.*`)
- for nextLogId == lastLogId {
+ count := 0
+ for ; nextLogId == lastLogId && count < 20; count++ {
time.Sleep(50 * time.Millisecond)
err = client.RequestAndDecode(&logentries, "GET", "arvados/v1/logs", nil,
arvados.ResourceListParams{
@@ -1229,7 +1230,7 @@ func (s *IntegrationSuite) checkUploadDownloadRequest(c *check.C, h *handler, re
nextLogId = logentries.Items[0].ID
}
}
-
+ c.Check(count, check.Not(check.Equals), 20)
c.Check(logentries.Items[0].ObjectUUID, check.Equals, userUuid)
c.Check(logentries.Items[0].Properties["collection_uuid"], check.Equals, collectionUuid)
c.Check(logentries.Items[0].Properties["collection_file_path"], check.Equals, filepath)
@@ -1244,6 +1245,8 @@ func (s *IntegrationSuite) TestDownloadLoggingPermission(c *check.C) {
h := handler{Config: config}
u := mustParseURL("http://" + arvadostest.FooCollection + ".keep-web.example/foo")
+ config.cluster.Collections.TrustAllContent = true
+
for _, adminperm := range []bool{true, false} {
for _, userperm := range []bool{true, false} {
config.cluster.Collections.WebDAVPermission.Admin.Download = adminperm
@@ -1276,6 +1279,38 @@ func (s *IntegrationSuite) TestDownloadLoggingPermission(c *check.C) {
arvadostest.ActiveUserUUID, arvadostest.FooCollection, "foo")
}
}
+
+ config.cluster.Collections.WebDAVPermission.User.Download = true
+
+ for _, tryurl := range []string{"http://" + arvadostest.MultilevelCollection1 + ".keep-web.example/dir1/subdir/file1",
+ "http://keep-web/users/active/multilevel_collection_1/dir1/subdir/file1"} {
+
+ u = mustParseURL(tryurl)
+ req := &http.Request{
+ Method: "GET",
+ Host: u.Host,
+ URL: u,
+ RequestURI: u.RequestURI(),
+ Header: http.Header{
+ "Authorization": {"Bearer " + arvadostest.ActiveToken},
+ },
+ }
+ s.checkUploadDownloadRequest(c, &h, req, http.StatusOK, "download", true,
+ arvadostest.ActiveUserUUID, arvadostest.MultilevelCollection1, "dir1/subdir/file1")
+ }
+
+ u = mustParseURL("http://" + strings.Replace(arvadostest.FooCollectionPDH, "+", "-", 1) + ".keep-web.example/foo")
+ req := &http.Request{
+ Method: "GET",
+ Host: u.Host,
+ URL: u,
+ RequestURI: u.RequestURI(),
+ Header: http.Header{
+ "Authorization": {"Bearer " + arvadostest.ActiveToken},
+ },
+ }
+ s.checkUploadDownloadRequest(c, &h, req, http.StatusOK, "download", true,
+ arvadostest.ActiveUserUUID, arvadostest.FooCollection, "foo")
}
func (s *IntegrationSuite) TestUploadLoggingPermission(c *check.C) {
commit 572e375e1c950f35d7e159bac031de5205354590
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Thu Jun 17 14:48:33 2021 -0400
17464: Clean up tests
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/services/keep-web/handler.go b/services/keep-web/handler.go
index 43ce57904..6d0b7669e 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</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 726c6220e..65dfb4312 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 357c42ae6..e6262374d 100644
--- 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 {
commit 046d4591c694537dc1a415744413986785fc6b9c
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Thu Jun 17 12:27:19 2021 -0400
17464: Update config variables & docs from feedback
Change KeepWebPermission -> WebDAVPermission
Add WebDAVLogEvents
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/doc/admin/restricting-upload-download.html.textile.liquid b/doc/admin/restricting-upload-download.html.textile.liquid
index 602cdfd61..45bcd8a07 100644
--- a/doc/admin/restricting-upload-download.html.textile.liquid
+++ b/doc/admin/restricting-upload-download.html.textile.liquid
@@ -14,56 +14,70 @@ For some use cases, you may want to limit the ability of users to upload or down
This feature exists in addition to the existing Arvados permission system. Users can only download from collections they have @read@ access to, and can only upload to projects and collections they have @write@ access to.
-h2. Keep-web and Keepproxy Permissions
-
There are two services involved in accessing data from outside the cluster.
- at keeproxy@ makes it possible to use @arv-put@ and @arv-get at . It works in terms of individual 64 MiB keep blocks. It prints a log each time a user uploads or downloads an individual block.
-
- at keep-web@ makes it possible to use Workbench, WebDAV and S3 API. It works in terms of individual files. It prints a log each time a user uploads or downloads a file, and also adds an entry into the API server @logs@ table.
-
-This distinction is important for auditing, the @keep-web@ records 'upload' and 'download' events on the API server that are included in the "User Activity Report":user-activity.html , whereas @keepprox@ only logs upload and download of individual blocks, which require a reverse lookup to determine the collection(s) and file(s) a block is associated with.
+h2. Keepproxy Permissions
-You can set permissions for @keep-web@ and @keepproxy@, with separate policies for regular users and admin users.
-
-If the user attempts to upload or download from a service without permission, they will receive a @403 Forbidden@ response. This only applies to file content. Users can still see collection listings.
+Permitting @keeproxy@ makes it possible to use @arv-put@ and @arv-get@, and upload from Workbench 1. It works in terms of individual 64 MiB keep blocks. It prints a log each time a user uploads or downloads an individual block.
The default policy allows anyone to upload or download.
<pre>
Collections:
- KeepWebPermisison:
+ KeepproxyPermission:
User:
Download: true
Upload: true
Admin:
Download: true
Upload: true
+</pre>
- KeepproxyPermission:
+h2. WebDAV and S3 API Permissions
+
+Permitting @WebDAV@ makes it possible to use WebDAV, S3 API, download from Workbench 1, and upload/download with Workbench 2. It works in terms of individual files. It prints a log each time a user uploads or downloads a file. When @WebDAVLogEvents@ (default true) is enabled, it also adds an entry into the API server @logs@ table.
+
+When a user attempts to upload or download from a service without permission, they will receive a @403 Forbidden@ response. This only applies to file content.
+
+Denying download permission does not deny access to access to XML file listings with PROPFIND. As a side effect it does deny auto-generated HTML documents that have file listings.
+
+Denying upload permission does not deny other operations that modify collections without directly accessing file content, such as MOVE and COPY.
+
+The default policy allows anyone to upload or download.
+
+<pre>
+ Collections:
+ WebDAVPermisison:
User:
Download: true
Upload: true
Admin:
Download: true
Upload: true
+ WebDAVLogEvents: true
</pre>
h2. Shell node and container permissions
-Be aware that even when upload and download from outside the network is not allowed, a user who has access to a shell node or runs a container still has internal access to Keep. (This is necessary to be able to run workflows). From the shell node or container, a user could send data outside the network by some other method, although this requires more intent than accidentally clicking on a link and downloading a file. It is possible to set up a firewall to prevent shell and compute nodes from making connections to hosts outside the private network. Exactly how to configure this is out of scope for this page, as it depends on the specific network infrastructure of your cluster.
+Be aware that even when upload and download from outside the network is not allowed, a user who has access to a shell node or runs a container still has internal access to Keep. (This is necessary to be able to run workflows). From the shell node or container, a user could send data outside the network by some other method, although this requires more intent than accidentally clicking on a link and downloading a file. It is possible to set up a firewall to prevent shell and compute nodes from making connections to hosts outside the private network. Exactly how to configure firewalls is out of scope for this page, as it depends on the specific network infrastructure of your cluster.
h2. Choosing a policy
+This distinction between WebDAV and Keepproxy is important for auditing. WebDAV records 'upload' and 'download' events on the API server that are included in the "User Activity Report":user-activity.html , whereas @keepproxy@ only logs upload and download of individual blocks, which require a reverse lookup to determine the collection(s) and file(s) a block is associated with.
+
+You set separate permissions for @WebDAV@ and @Keepproxy@, with separate policies for regular users and admin users.
+
These policies apply to only access from outside the cluster, using Workbench or Arvados CLI tools.
+The @WebDAVLogEvents@ option should be enabled if you intend to the run the "User Activity Report":user-activity.html . If you don't need audits, or you are running a site that is mostly serving public data to anonymous downloaders, you can disable in to avoid the extra API server request.
+
h3. Audited downloads
-For ease of access auditing, this policy prevents downloads using @arv-get at . Downloads through @keep-web@ are permitted, but logged. Uploads with @arv-put@ are allowed.
+For ease of access auditing, this policy prevents downloads using @arv-get at . Downloads through WebDAV and S3 API are permitted, but logged. Uploads are allowed.
<pre>
Collections:
- KeepWebPermisison:
+ WebDAVPermisison:
User:
Download: true
Upload: true
@@ -78,6 +92,7 @@ For ease of access auditing, this policy prevents downloads using @arv-get at . Do
Admin:
Download: false
Upload: true
+ WebDAVLogEvents: true
</pre>
h3. Disallow downloads by regular users
@@ -86,7 +101,7 @@ This policy prevents regular users (non-admin) from downloading data. Uploading
<pre>
Collections:
- KeepWebPermisison:
+ WebDAVPermisison:
User:
Download: false
Upload: true
@@ -101,6 +116,7 @@ This policy prevents regular users (non-admin) from downloading data. Uploading
Admin:
Download: true
Upload: true
+ WebDAVLogEvents: true
</pre>
h3. Disallow uploads by regular users
@@ -109,7 +125,7 @@ This policy is suitable for an installation where data is being shared with a gr
<pre>
Collections:
- KeepWebPermisison:
+ WebDAVPermisison:
User:
Download: true
Upload: false
@@ -124,4 +140,5 @@ This policy is suitable for an installation where data is being shared with a gr
Admin:
Download: true
Upload: true
+ WebDAVLogEvents: true
</pre>
diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index 537db6762..ff2f649fa 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -554,11 +554,10 @@ Clusters:
# Persistent sessions.
MaxSessions: 100
- # Selectively set permissions for regular users and admins to be
- # able to download or upload data files using the
- # upload/download features for Workbench, WebDAV and S3 API
- # support.
- KeepWebPermission:
+ # Selectively set permissions for regular users and admins to
+ # download or upload data files using the upload/download
+ # features for Workbench, WebDAV and S3 API support.
+ WebDAVPermission:
User:
Download: true
Upload: true
@@ -577,6 +576,12 @@ Clusters:
Download: true
Upload: true
+ # Post upload / download events to the API server logs table, so
+ # that they can be included in the arv-user-activity report.
+ # You can disable this if you find that it is creating excess
+ # load on the API server and you don't need it.
+ WebDAVLogEvents: true
+
Login:
# One of the following mechanisms (SSO, Google, PAM, LDAP, or
# LoginCluster) should be enabled; see
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index e2ba3bc2c..34cc07115 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -560,11 +560,10 @@ Clusters:
# Persistent sessions.
MaxSessions: 100
- # Selectively set permissions for regular users and admins to be
- # able to download or upload data files using the
- # upload/download features for Workbench, WebDAV and S3 API
- # support.
- KeepWebPermission:
+ # Selectively set permissions for regular users and admins to
+ # download or upload data files using the upload/download
+ # features for Workbench, WebDAV and S3 API support.
+ WebDAVPermission:
User:
Download: true
Upload: true
@@ -583,6 +582,12 @@ Clusters:
Download: true
Upload: true
+ # Post upload / download events to the API server logs table, so
+ # that they can be included in the arv-user-activity report.
+ # You can disable this if you find that it is creating excess
+ # load on the API server and you don't need it.
+ WebDAVLogEvents: true
+
Login:
# One of the following mechanisms (SSO, Google, PAM, LDAP, or
# LoginCluster) should be enabled; see
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index 83a670832..13fe989ca 100644
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -142,7 +142,8 @@ type Cluster struct {
WebDAVCache WebDAVCacheConfig
KeepproxyPermission UploadDownloadRolePermissions
- KeepWebPermission UploadDownloadRolePermissions
+ WebDAVPermission UploadDownloadRolePermissions
+ WebDAVLogEvents bool
}
Git struct {
GitCommand string
commit 154ba68b8f66e50d9ddb53f3a5f19890588f42f4
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Wed Jun 16 17:22:04 2021 -0400
17464: Activity report lists upload/download events
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/tools/user-activity/arvados_user_activity/main.py b/tools/user-activity/arvados_user_activity/main.py
index 959f16d89..997da57e0 100755
--- a/tools/user-activity/arvados_user_activity/main.py
+++ b/tools/user-activity/arvados_user_activity/main.py
@@ -41,6 +41,13 @@ def getuserinfo(arv, uuid):
arv.config()["Services"]["Workbench1"]["ExternalURL"],
uuid, prof)
+collectionNameCache = {}
+def getCollectionName(arv, uuid):
+ if uuid not in collectionNameCache:
+ u = arv.collections().get(uuid=uuid).execute()
+ collectionNameCache[uuid] = u["name"]
+ return collectionNameCache[uuid]
+
def getname(u):
return "\"%s\" (%s)" % (u["name"], u["uuid"])
@@ -137,6 +144,19 @@ def main(arguments=None):
else:
users[owner].append("%s Deleted collection %s %s" % (event_at, getname(e["properties"]["old_attributes"]), loguuid))
+ elif e["event_type"] == "file_download":
+ users[e["object_uuid"]].append("%s Downloaded file \"%s\" from \"%s\" (%s) (%s)" % (event_at,
+ e["properties"].get("collection_file_path") or e["properties"].get("reqPath"),
+ getCollectionName(arv, e["properties"].get("collection_uuid")),
+ e["properties"].get("collection_uuid"),
+ e["properties"].get("portable_data_hash")))
+
+ elif e["event_type"] == "file_upload":
+ users[e["object_uuid"]].append("%s Uploaded file \"%s\" to \"%s\" (%s)" % (event_at,
+ e["properties"].get("collection_file_path") or e["properties"].get("reqPath"),
+ getCollectionName(arv, e["properties"].get("collection_uuid")),
+ e["properties"].get("collection_uuid")))
+
else:
users[owner].append("%s %s %s %s" % (e["event_type"], e["object_kind"], e["object_uuid"], loguuid))
commit a271aaf8e345f8035a8818cee15868bd1c38ceb0
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Wed Jun 16 16:06:36 2021 -0400
17464: Fix test.
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/services/keepproxy/keepproxy_test.go b/services/keepproxy/keepproxy_test.go
index 9cf5ded89..c5d94f9ac 100644
--- a/services/keepproxy/keepproxy_test.go
+++ b/services/keepproxy/keepproxy_test.go
@@ -720,11 +720,11 @@ func (s *ServerRequiredSuite) TestPutAskGetInvalidToken(c *C) {
_, _, _, err = kc.Get(hash)
c.Assert(err, FitsTypeOf, &keepclient.ErrNotFound{})
c.Check(err.(*keepclient.ErrNotFound).Temporary(), Equals, false)
- c.Check(err, ErrorMatches, ".*HTTP 403 \"Missing or invalid Authorization header\".*")
+ c.Check(err, ErrorMatches, ".*HTTP 403 \"Missing or invalid Authorization header, or method not allowed\".*")
}
_, _, err = kc.PutB([]byte("foo"))
- c.Check(err, ErrorMatches, ".*403.*Missing or invalid Authorization header")
+ c.Check(err, ErrorMatches, ".*403.*Missing or invalid Authorization header, or method not allowed")
}
}
commit e093e9ea2e7c204c5c58de006d8b316c9013b6dd
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Wed Jun 16 16:03:48 2021 -0400
17464: Add upload/download logging and permissions to keepproxy
Add tests.
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/services/keepproxy/keepproxy.go b/services/keepproxy/keepproxy.go
index 538a06122..df6e06a74 100644
--- a/services/keepproxy/keepproxy.go
+++ b/services/keepproxy/keepproxy.go
@@ -163,19 +163,20 @@ func run(logger log.FieldLogger, cluster *arvados.Cluster) error {
signal.Notify(term, syscall.SIGINT)
// Start serving requests.
- router = MakeRESTRouter(kc, time.Duration(keepclient.DefaultProxyRequestTimeout), cluster.ManagementToken)
+ router = MakeRESTRouter(kc, time.Duration(keepclient.DefaultProxyRequestTimeout), cluster, logger)
return http.Serve(listener, httpserver.AddRequestIDs(httpserver.LogRequests(router)))
}
type APITokenCache struct {
tokens map[string]int64
+ tokenUser map[string]*arvados.User
lock sync.Mutex
expireTime int64
}
// RememberToken caches the token and set an expire time. If we already have
// an expire time on the token, it is not updated.
-func (cache *APITokenCache) RememberToken(token string) {
+func (cache *APITokenCache) RememberToken(token string, user *arvados.User) {
cache.lock.Lock()
defer cache.lock.Unlock()
@@ -183,25 +184,26 @@ func (cache *APITokenCache) RememberToken(token string) {
if cache.tokens[token] == 0 {
cache.tokens[token] = now + cache.expireTime
}
+ cache.tokenUser[token] = user
}
// RecallToken checks if the cached token is known and still believed to be
// valid.
-func (cache *APITokenCache) RecallToken(token string) bool {
+func (cache *APITokenCache) RecallToken(token string) (bool, *arvados.User) {
cache.lock.Lock()
defer cache.lock.Unlock()
now := time.Now().Unix()
if cache.tokens[token] == 0 {
// Unknown token
- return false
+ return false, nil
} else if now < cache.tokens[token] {
// Token is known and still valid
- return true
+ return true, cache.tokenUser[token]
} else {
// Token is expired
cache.tokens[token] = 0
- return false
+ return false, nil
}
}
@@ -216,10 +218,10 @@ func GetRemoteAddress(req *http.Request) string {
return req.RemoteAddr
}
-func CheckAuthorizationHeader(kc *keepclient.KeepClient, cache *APITokenCache, req *http.Request) (pass bool, tok string) {
+func (h *proxyHandler) CheckAuthorizationHeader(req *http.Request) (pass bool, tok string, user *arvados.User) {
parts := strings.SplitN(req.Header.Get("Authorization"), " ", 2)
if len(parts) < 2 || !(parts[0] == "OAuth2" || parts[0] == "Bearer") || len(parts[1]) == 0 {
- return false, ""
+ return false, "", nil
}
tok = parts[1]
@@ -234,29 +236,52 @@ func CheckAuthorizationHeader(kc *keepclient.KeepClient, cache *APITokenCache, r
op = "write"
}
- if cache.RecallToken(op + ":" + tok) {
+ if ok, user := h.APITokenCache.RecallToken(op + ":" + tok); ok {
// Valid in the cache, short circuit
- return true, tok
+ return true, tok, user
}
var err error
- arv := *kc.Arvados
+ arv := *h.KeepClient.Arvados
arv.ApiToken = tok
arv.RequestID = req.Header.Get("X-Request-Id")
+ user = &arvados.User{}
+ userCurrentError := arv.Call("GET", "users", "", "current", nil, user)
if op == "read" {
+ // scoped token this will fail the user current check,
+ // but if it is a download operation and they can read
+ // the keep_services table, it's okay.
err = arv.Call("HEAD", "keep_services", "", "accessible", nil, nil)
} else {
- err = arv.Call("HEAD", "users", "", "current", nil, nil)
+ err = userCurrentError
}
if err != nil {
log.Printf("%s: CheckAuthorizationHeader error: %v", GetRemoteAddress(req), err)
- return false, ""
+ return false, "", nil
+ }
+
+ if userCurrentError == nil && user.IsAdmin {
+ // checking userCurrentError is probably redundant,
+ // IsAdmin would be false anyway. But can't hurt.
+ if op == "read" && !h.cluster.Collections.KeepproxyPermission.Admin.Download {
+ return false, "", nil
+ }
+ if op == "write" && !h.cluster.Collections.KeepproxyPermission.Admin.Upload {
+ return false, "", nil
+ }
+ } else {
+ if op == "read" && !h.cluster.Collections.KeepproxyPermission.User.Download {
+ return false, "", nil
+ }
+ if op == "write" && !h.cluster.Collections.KeepproxyPermission.User.Upload {
+ return false, "", nil
+ }
}
// Success! Update cache
- cache.RememberToken(op + ":" + tok)
+ h.APITokenCache.RememberToken(op+":"+tok, user)
- return true, tok
+ return true, tok, user
}
// We need to make a private copy of the default http transport early
@@ -273,11 +298,13 @@ type proxyHandler struct {
*APITokenCache
timeout time.Duration
transport *http.Transport
+ logger log.FieldLogger
+ cluster *arvados.Cluster
}
// MakeRESTRouter returns an http.Handler that passes GET and PUT
// requests to the appropriate handlers.
-func MakeRESTRouter(kc *keepclient.KeepClient, timeout time.Duration, mgmtToken string) http.Handler {
+func MakeRESTRouter(kc *keepclient.KeepClient, timeout time.Duration, cluster *arvados.Cluster, logger log.FieldLogger) http.Handler {
rest := mux.NewRouter()
transport := defaultTransport
@@ -296,8 +323,11 @@ func MakeRESTRouter(kc *keepclient.KeepClient, timeout time.Duration, mgmtToken
transport: &transport,
APITokenCache: &APITokenCache{
tokens: make(map[string]int64),
+ tokenUser: make(map[string]*arvados.User),
expireTime: 300,
},
+ logger: logger,
+ cluster: cluster,
}
rest.HandleFunc(`/{locator:[0-9a-f]{32}\+.*}`, h.Get).Methods("GET", "HEAD")
@@ -316,7 +346,7 @@ func MakeRESTRouter(kc *keepclient.KeepClient, timeout time.Duration, mgmtToken
rest.HandleFunc(`/`, h.Options).Methods("OPTIONS")
rest.Handle("/_health/{check}", &health.Handler{
- Token: mgmtToken,
+ Token: cluster.ManagementToken,
Prefix: "/_health/",
}).Methods("GET")
@@ -326,9 +356,9 @@ func MakeRESTRouter(kc *keepclient.KeepClient, timeout time.Duration, mgmtToken
var errLoopDetected = errors.New("loop detected")
-func (*proxyHandler) checkLoop(resp http.ResponseWriter, req *http.Request) error {
+func (h *proxyHandler) checkLoop(resp http.ResponseWriter, req *http.Request) error {
if via := req.Header.Get("Via"); strings.Index(via, " "+viaAlias) >= 0 {
- log.Printf("proxy loop detected (request has Via: %q): perhaps keepproxy is misidentified by gateway config as an external client, or its keep_services record does not have service_type=proxy?", via)
+ h.logger.Printf("proxy loop detected (request has Via: %q): perhaps keepproxy is misidentified by gateway config as an external client, or its keep_services record does not have service_type=proxy?", via)
http.Error(resp, errLoopDetected.Error(), http.StatusInternalServerError)
return errLoopDetected
}
@@ -354,7 +384,7 @@ func (h *proxyHandler) Options(resp http.ResponseWriter, req *http.Request) {
SetCorsHeaders(resp)
}
-var errBadAuthorizationHeader = errors.New("Missing or invalid Authorization header")
+var errBadAuthorizationHeader = errors.New("Missing or invalid Authorization header, or method not allowed")
var errContentLengthMismatch = errors.New("Actual length != expected content length")
var errMethodNotSupported = errors.New("Method not supported")
@@ -384,7 +414,8 @@ func (h *proxyHandler) Get(resp http.ResponseWriter, req *http.Request) {
var pass bool
var tok string
- if pass, tok = CheckAuthorizationHeader(kc, h.APITokenCache, req); !pass {
+ var user *arvados.User
+ if pass, tok, user = h.CheckAuthorizationHeader(req); !pass {
status, err = http.StatusForbidden, errBadAuthorizationHeader
return
}
@@ -398,6 +429,18 @@ func (h *proxyHandler) Get(resp http.ResponseWriter, req *http.Request) {
locator = removeHint.ReplaceAllString(locator, "$1")
+ if locator != "" {
+ parts := strings.SplitN(locator, "+", 3)
+ if len(parts) >= 2 {
+ logger := h.logger
+ if user != nil {
+ logger = logger.WithField("user_uuid", user.UUID).
+ WithField("user_full_name", user.FullName)
+ }
+ logger.WithField("locator", fmt.Sprintf("%s+%s", parts[0], parts[1])).Infof("Block download")
+ }
+ }
+
switch req.Method {
case "HEAD":
expectLength, proxiedURI, err = kc.Ask(locator)
@@ -498,7 +541,8 @@ func (h *proxyHandler) Put(resp http.ResponseWriter, req *http.Request) {
var pass bool
var tok string
- if pass, tok = CheckAuthorizationHeader(kc, h.APITokenCache, req); !pass {
+ var user *arvados.User
+ if pass, tok, user = h.CheckAuthorizationHeader(req); !pass {
err = errBadAuthorizationHeader
status = http.StatusForbidden
return
@@ -531,6 +575,18 @@ func (h *proxyHandler) Put(resp http.ResponseWriter, req *http.Request) {
locatorOut, wroteReplicas, err = kc.PutHR(locatorIn, req.Body, expectLength)
}
+ if locatorOut != "" {
+ parts := strings.SplitN(locatorOut, "+", 3)
+ if len(parts) >= 2 {
+ logger := h.logger
+ if user != nil {
+ logger = logger.WithField("user_uuid", user.UUID).
+ WithField("user_full_name", user.FullName)
+ }
+ logger.WithField("locator", fmt.Sprintf("%s+%s", parts[0], parts[1])).Infof("Block upload")
+ }
+ }
+
// Tell the client how many successful PUTs we accomplished
resp.Header().Set(keepclient.XKeepReplicasStored, fmt.Sprintf("%d", wroteReplicas))
@@ -580,7 +636,7 @@ func (h *proxyHandler) Index(resp http.ResponseWriter, req *http.Request) {
}()
kc := h.makeKeepClient(req)
- ok, token := CheckAuthorizationHeader(kc, h.APITokenCache, req)
+ ok, token, _ := h.CheckAuthorizationHeader(req)
if !ok {
status, err = http.StatusForbidden, errBadAuthorizationHeader
return
diff --git a/services/keepproxy/keepproxy_test.go b/services/keepproxy/keepproxy_test.go
index 6a02ab9bd..9cf5ded89 100644
--- a/services/keepproxy/keepproxy_test.go
+++ b/services/keepproxy/keepproxy_test.go
@@ -120,7 +120,7 @@ func (s *NoKeepServerSuite) TearDownSuite(c *C) {
arvadostest.StopAPI()
}
-func runProxy(c *C, bogusClientToken bool, loadKeepstoresFromConfig bool) *keepclient.KeepClient {
+func runProxy(c *C, bogusClientToken bool, loadKeepstoresFromConfig bool, kp *arvados.UploadDownloadRolePermissions) (*keepclient.KeepClient, *bytes.Buffer) {
cfg, err := config.NewLoader(nil, ctxlog.TestLogger(c)).Load()
c.Assert(err, Equals, nil)
cluster, err := cfg.GetCluster("")
@@ -133,9 +133,16 @@ func runProxy(c *C, bogusClientToken bool, loadKeepstoresFromConfig bool) *keepc
cluster.Services.Keepproxy.InternalURLs = map[arvados.URL]arvados.ServiceInstance{{Host: ":0"}: {}}
+ if kp != nil {
+ cluster.Collections.KeepproxyPermission = *kp
+ }
+
listener = nil
+ logbuf := &bytes.Buffer{}
+ logger := log.New()
+ logger.Out = logbuf
go func() {
- run(log.New(), cluster)
+ run(logger, cluster)
defer closeListener()
}()
waitForListener()
@@ -153,11 +160,11 @@ func runProxy(c *C, bogusClientToken bool, loadKeepstoresFromConfig bool) *keepc
kc.SetServiceRoots(sr, sr, sr)
kc.Arvados.External = true
- return kc
+ return kc, logbuf
}
func (s *ServerRequiredSuite) TestResponseViaHeader(c *C) {
- runProxy(c, false, false)
+ runProxy(c, false, false, nil)
defer closeListener()
req, err := http.NewRequest("POST",
@@ -184,7 +191,7 @@ func (s *ServerRequiredSuite) TestResponseViaHeader(c *C) {
}
func (s *ServerRequiredSuite) TestLoopDetection(c *C) {
- kc := runProxy(c, false, false)
+ kc, _ := runProxy(c, false, false, nil)
defer closeListener()
sr := map[string]string{
@@ -202,7 +209,7 @@ func (s *ServerRequiredSuite) TestLoopDetection(c *C) {
}
func (s *ServerRequiredSuite) TestStorageClassesHeader(c *C) {
- kc := runProxy(c, false, false)
+ kc, _ := runProxy(c, false, false, nil)
defer closeListener()
// Set up fake keepstore to record request headers
@@ -229,7 +236,7 @@ func (s *ServerRequiredSuite) TestStorageClassesHeader(c *C) {
}
func (s *ServerRequiredSuite) TestDesiredReplicas(c *C) {
- kc := runProxy(c, false, false)
+ kc, _ := runProxy(c, false, false, nil)
defer closeListener()
content := []byte("TestDesiredReplicas")
@@ -246,7 +253,7 @@ func (s *ServerRequiredSuite) TestDesiredReplicas(c *C) {
}
func (s *ServerRequiredSuite) TestPutWrongContentLength(c *C) {
- kc := runProxy(c, false, false)
+ kc, _ := runProxy(c, false, false, nil)
defer closeListener()
content := []byte("TestPutWrongContentLength")
@@ -257,7 +264,7 @@ func (s *ServerRequiredSuite) TestPutWrongContentLength(c *C) {
// fixes the invalid Content-Length header. In order to test
// our server behavior, we have to call the handler directly
// using an httptest.ResponseRecorder.
- rtr := MakeRESTRouter(kc, 10*time.Second, "")
+ rtr := MakeRESTRouter(kc, 10*time.Second, &arvados.Cluster{}, log.New())
type testcase struct {
sendLength string
@@ -285,7 +292,7 @@ func (s *ServerRequiredSuite) TestPutWrongContentLength(c *C) {
}
func (s *ServerRequiredSuite) TestManyFailedPuts(c *C) {
- kc := runProxy(c, false, false)
+ kc, _ := runProxy(c, false, false, nil)
defer closeListener()
router.(*proxyHandler).timeout = time.Nanosecond
@@ -312,7 +319,7 @@ func (s *ServerRequiredSuite) TestManyFailedPuts(c *C) {
}
func (s *ServerRequiredSuite) TestPutAskGet(c *C) {
- kc := runProxy(c, false, false)
+ kc, logbuf := runProxy(c, false, false, nil)
defer closeListener()
hash := fmt.Sprintf("%x", md5.Sum([]byte("foo")))
@@ -348,6 +355,9 @@ func (s *ServerRequiredSuite) TestPutAskGet(c *C) {
c.Check(rep, Equals, 2)
c.Check(err, Equals, nil)
c.Log("Finished PutB (expected success)")
+
+ c.Check(logbuf.String(), Matches, `(?ms).*msg="Block upload" locator=acbd18db4cc2f85cedef654fccc4a4d8\+3 user_full_name="TestCase Administrator" user_uuid=zzzzz-tpzed-d9tiejq69daie8f.*`)
+ logbuf.Reset()
}
{
@@ -355,6 +365,8 @@ func (s *ServerRequiredSuite) TestPutAskGet(c *C) {
c.Assert(err, Equals, nil)
c.Check(blocklen, Equals, int64(3))
c.Log("Finished Ask (expected success)")
+ c.Check(logbuf.String(), Matches, `(?ms).*msg="Block download" locator=acbd18db4cc2f85cedef654fccc4a4d8\+3 user_full_name="TestCase Administrator" user_uuid=zzzzz-tpzed-d9tiejq69daie8f.*`)
+ logbuf.Reset()
}
{
@@ -365,6 +377,8 @@ func (s *ServerRequiredSuite) TestPutAskGet(c *C) {
c.Check(all, DeepEquals, []byte("foo"))
c.Check(blocklen, Equals, int64(3))
c.Log("Finished Get (expected success)")
+ c.Check(logbuf.String(), Matches, `(?ms).*msg="Block download" locator=acbd18db4cc2f85cedef654fccc4a4d8\+3 user_full_name="TestCase Administrator" user_uuid=zzzzz-tpzed-d9tiejq69daie8f.*`)
+ logbuf.Reset()
}
{
@@ -389,7 +403,7 @@ func (s *ServerRequiredSuite) TestPutAskGet(c *C) {
}
func (s *ServerRequiredSuite) TestPutAskGetForbidden(c *C) {
- kc := runProxy(c, true, false)
+ kc, _ := runProxy(c, true, false, nil)
defer closeListener()
hash := fmt.Sprintf("%x+3", md5.Sum([]byte("bar")))
@@ -411,11 +425,109 @@ func (s *ServerRequiredSuite) TestPutAskGetForbidden(c *C) {
c.Check(err, FitsTypeOf, &keepclient.ErrNotFound{})
c.Check(err, ErrorMatches, ".*not found.*")
c.Check(blocklen, Equals, int64(0))
+}
+
+func testPermission(c *C, admin bool, perm arvados.UploadDownloadPermission) {
+ kp := arvados.UploadDownloadRolePermissions{}
+ if admin {
+ kp.Admin = perm
+ kp.User = arvados.UploadDownloadPermission{Upload: true, Download: true}
+ } else {
+ kp.Admin = arvados.UploadDownloadPermission{Upload: true, Download: true}
+ kp.User = perm
+ }
+
+ kc, logbuf := runProxy(c, false, false, &kp)
+ defer closeListener()
+ if admin {
+ kc.Arvados.ApiToken = arvadostest.AdminToken
+ } else {
+ kc.Arvados.ApiToken = arvadostest.ActiveToken
+ }
+
+ hash := fmt.Sprintf("%x", md5.Sum([]byte("foo")))
+ var hash2 string
+
+ {
+ var rep int
+ var err error
+ hash2, rep, err = kc.PutB([]byte("foo"))
+ if perm.Upload {
+ c.Check(hash2, Matches, fmt.Sprintf(`^%s\+3(\+.+)?$`, hash))
+ c.Check(rep, Equals, 2)
+ c.Check(err, Equals, nil)
+ c.Log("Finished PutB (expected success)")
+ if admin {
+ c.Check(logbuf.String(), Matches, `(?ms).*msg="Block upload" locator=acbd18db4cc2f85cedef654fccc4a4d8\+3 user_full_name="TestCase Administrator" user_uuid=zzzzz-tpzed-d9tiejq69daie8f.*`)
+ } else {
+
+ c.Check(logbuf.String(), Matches, `(?ms).*msg="Block upload" locator=acbd18db4cc2f85cedef654fccc4a4d8\+3 user_full_name="Active User" user_uuid=zzzzz-tpzed-xurymjxw79nv3jz.*`)
+ }
+ } else {
+ c.Check(hash2, Equals, "")
+ c.Check(rep, Equals, 0)
+ c.Check(err, FitsTypeOf, keepclient.InsufficientReplicasError(errors.New("")))
+ }
+ logbuf.Reset()
+ }
+ if perm.Upload {
+ // can't test download without upload.
+
+ reader, blocklen, _, err := kc.Get(hash2)
+ if perm.Download {
+ c.Assert(err, Equals, nil)
+ all, err := ioutil.ReadAll(reader)
+ c.Check(err, IsNil)
+ c.Check(all, DeepEquals, []byte("foo"))
+ c.Check(blocklen, Equals, int64(3))
+ c.Log("Finished Get (expected success)")
+ if admin {
+ c.Check(logbuf.String(), Matches, `(?ms).*msg="Block download" locator=acbd18db4cc2f85cedef654fccc4a4d8\+3 user_full_name="TestCase Administrator" user_uuid=zzzzz-tpzed-d9tiejq69daie8f.*`)
+ } else {
+ c.Check(logbuf.String(), Matches, `(?ms).*msg="Block download" locator=acbd18db4cc2f85cedef654fccc4a4d8\+3 user_full_name="Active User" user_uuid=zzzzz-tpzed-xurymjxw79nv3jz.*`)
+ }
+ } else {
+ c.Check(err, FitsTypeOf, &keepclient.ErrNotFound{})
+ c.Check(err, ErrorMatches, ".*Missing or invalid Authorization header, or method not allowed.*")
+ c.Check(blocklen, Equals, int64(0))
+ }
+ logbuf.Reset()
+ }
+
+}
+
+func (s *ServerRequiredSuite) TestPutGetPermission(c *C) {
+
+ for _, adminperm := range []bool{true, false} {
+ for _, userperm := range []bool{true, false} {
+
+ testPermission(c, true,
+ arvados.UploadDownloadPermission{
+ Upload: adminperm,
+ Download: true,
+ })
+ testPermission(c, true,
+ arvados.UploadDownloadPermission{
+ Upload: true,
+ Download: adminperm,
+ })
+ testPermission(c, false,
+ arvados.UploadDownloadPermission{
+ Upload: true,
+ Download: userperm,
+ })
+ testPermission(c, false,
+ arvados.UploadDownloadPermission{
+ Upload: true,
+ Download: userperm,
+ })
+ }
+ }
}
func (s *ServerRequiredSuite) TestCorsHeaders(c *C) {
- runProxy(c, false, false)
+ runProxy(c, false, false, nil)
defer closeListener()
{
@@ -446,7 +558,7 @@ func (s *ServerRequiredSuite) TestCorsHeaders(c *C) {
}
func (s *ServerRequiredSuite) TestPostWithoutHash(c *C) {
- runProxy(c, false, false)
+ runProxy(c, false, false, nil)
defer closeListener()
{
@@ -504,7 +616,7 @@ func (s *ServerRequiredConfigYmlSuite) TestGetIndex(c *C) {
}
func getIndexWorker(c *C, useConfig bool) {
- kc := runProxy(c, false, useConfig)
+ kc, _ := runProxy(c, false, useConfig, nil)
defer closeListener()
// Put "index-data" blocks
@@ -567,7 +679,7 @@ func getIndexWorker(c *C, useConfig bool) {
}
func (s *ServerRequiredSuite) TestCollectionSharingToken(c *C) {
- kc := runProxy(c, false, false)
+ kc, _ := runProxy(c, false, false, nil)
defer closeListener()
hash, _, err := kc.PutB([]byte("shareddata"))
c.Check(err, IsNil)
@@ -580,7 +692,7 @@ func (s *ServerRequiredSuite) TestCollectionSharingToken(c *C) {
}
func (s *ServerRequiredSuite) TestPutAskGetInvalidToken(c *C) {
- kc := runProxy(c, false, false)
+ kc, _ := runProxy(c, false, false, nil)
defer closeListener()
// Put a test block
@@ -617,7 +729,7 @@ func (s *ServerRequiredSuite) TestPutAskGetInvalidToken(c *C) {
}
func (s *ServerRequiredSuite) TestAskGetKeepProxyConnectionError(c *C) {
- kc := runProxy(c, false, false)
+ kc, _ := runProxy(c, false, false, nil)
defer closeListener()
// Point keepproxy at a non-existent keepstore
@@ -643,7 +755,7 @@ func (s *ServerRequiredSuite) TestAskGetKeepProxyConnectionError(c *C) {
}
func (s *NoKeepServerSuite) TestAskGetNoKeepServerError(c *C) {
- kc := runProxy(c, false, false)
+ kc, _ := runProxy(c, false, false, nil)
defer closeListener()
hash := fmt.Sprintf("%x", md5.Sum([]byte("foo")))
@@ -666,10 +778,10 @@ func (s *NoKeepServerSuite) TestAskGetNoKeepServerError(c *C) {
}
func (s *ServerRequiredSuite) TestPing(c *C) {
- kc := runProxy(c, false, false)
+ kc, _ := runProxy(c, false, false, nil)
defer closeListener()
- rtr := MakeRESTRouter(kc, 10*time.Second, arvadostest.ManagementToken)
+ rtr := MakeRESTRouter(kc, 10*time.Second, &arvados.Cluster{ManagementToken: arvadostest.ManagementToken}, log.New())
req, err := http.NewRequest("GET",
"http://"+listener.Addr().String()+"/_health/ping",
commit 7ab068369174b2a4de6251202a021f20ca1f36f3
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Tue Jun 15 16:59:40 2021 -0400
17464: Refactor tests and check that log events are posted
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/sdk/go/arvadostest/fixtures.go b/sdk/go/arvadostest/fixtures.go
index a4d7e88b2..0c05441a8 100644
--- a/sdk/go/arvadostest/fixtures.go
+++ b/sdk/go/arvadostest/fixtures.go
@@ -10,6 +10,7 @@ const (
ActiveToken = "3kg6k6lzmp9kj5cpkcoxie963cmvjahbt2fod9zru30k1jqdmi"
ActiveTokenUUID = "zzzzz-gj3su-077z32aux8dg2s1"
ActiveTokenV2 = "v2/zzzzz-gj3su-077z32aux8dg2s1/3kg6k6lzmp9kj5cpkcoxie963cmvjahbt2fod9zru30k1jqdmi"
+ AdminUserUUID = "zzzzz-tpzed-d9tiejq69daie8f"
AdminToken = "4axaw8zxe0qm22wa6urpp5nskcne8z88cvbupv653y1njyi05h"
AdminTokenUUID = "zzzzz-gj3su-027z32aux8dg2s1"
AnonymousToken = "4kg6k6lzmp9kj4cpkcoxie964cmvjahbt4fod9zru44k4jqdmi"
diff --git a/services/keep-web/handler_test.go b/services/keep-web/handler_test.go
index ba416d7cd..726c6220e 100644
--- a/services/keep-web/handler_test.go
+++ b/services/keep-web/handler_test.go
@@ -1188,18 +1188,21 @@ func copyHeader(h http.Header) http.Header {
return hc
}
-func (s *IntegrationSuite) TestDownloadLogging(c *check.C) {
- h := handler{Config: newConfig(s.ArvConfig)}
- u := mustParseURL("http://" + arvadostest.FooCollection + ".keep-web.example/foo")
- req := &http.Request{
- Method: "GET",
- Host: u.Host,
- URL: u,
- RequestURI: u.RequestURI(),
- Header: http.Header{
- "Authorization": {"Bearer " + arvadostest.ActiveToken},
- },
- }
+func (s *IntegrationSuite) checkUploadDownloadRequest(c *check.C, h *handler, req *http.Request,
+ successCode int, direction string, perm bool, userUuid string, collectionUuid string, filepath string) {
+
+ client := s.testServer.Config.Client
+ client.AuthToken = arvadostest.AdminToken
+ var logentries arvados.LogList
+ limit1 := 1
+ err := client.RequestAndDecode(&logentries, "GET", "arvados/v1/logs", nil,
+ arvados.ResourceListParams{
+ Limit: &limit1,
+ Order: "created_at desc"})
+ c.Check(err, check.IsNil)
+ c.Check(logentries.Items, check.HasLen, 1)
+ lastLogId := logentries.Items[0].ID
+ nextLogId := lastLogId
var logbuf bytes.Buffer
logger := logrus.New()
@@ -1208,49 +1211,41 @@ func (s *IntegrationSuite) TestDownloadLogging(c *check.C) {
req = req.WithContext(ctxlog.Context(context.Background(), logger))
h.ServeHTTP(resp, req)
- c.Check(logbuf.String(), check.Matches, `(?ms).*msg="File download".*`)
- c.Check(logbuf.String(), check.Not(check.Matches), `(?ms).*level=error.*`)
-}
-
-func (s *IntegrationSuite) TestUploadLogging(c *check.C) {
- defer func() {
- client := s.testServer.Config.Client
- client.AuthToken = arvadostest.AdminToken
- client.RequestAndDecode(nil, "POST", "database/reset", nil, nil)
- }()
+ if perm {
+ c.Check(resp.Result().StatusCode, check.Equals, successCode)
+ c.Check(logbuf.String(), check.Matches, `(?ms).*msg="File `+direction+`".*`)
+ c.Check(logbuf.String(), check.Not(check.Matches), `(?ms).*level=error.*`)
+
+ for nextLogId == lastLogId {
+ time.Sleep(50 * time.Millisecond)
+ err = client.RequestAndDecode(&logentries, "GET", "arvados/v1/logs", nil,
+ arvados.ResourceListParams{
+ Filters: []arvados.Filter{arvados.Filter{Attr: "event_type", Operator: "=", Operand: "file_" + direction}},
+ Limit: &limit1,
+ Order: "created_at desc",
+ })
+ c.Check(err, check.IsNil)
+ if len(logentries.Items) > 0 {
+ nextLogId = logentries.Items[0].ID
+ }
+ }
- h := handler{Config: newConfig(s.ArvConfig)}
- u := mustParseURL("http://" + arvadostest.FooCollection + ".keep-web.example/bar")
- req := &http.Request{
- Method: "PUT",
- Host: u.Host,
- URL: u,
- RequestURI: u.RequestURI(),
- Header: http.Header{
- "Authorization": {"Bearer " + arvadostest.ActiveToken},
- },
- Body: io.NopCloser(bytes.NewReader([]byte("bar"))),
+ c.Check(logentries.Items[0].ObjectUUID, check.Equals, userUuid)
+ c.Check(logentries.Items[0].Properties["collection_uuid"], check.Equals, collectionUuid)
+ c.Check(logentries.Items[0].Properties["collection_file_path"], check.Equals, filepath)
+ } else {
+ c.Check(resp.Result().StatusCode, check.Equals, http.StatusForbidden)
+ c.Check(logbuf.String(), check.Equals, "")
}
-
- var logbuf bytes.Buffer
- logger := logrus.New()
- logger.Out = &logbuf
- resp := httptest.NewRecorder()
- req = req.WithContext(ctxlog.Context(context.Background(), logger))
- h.ServeHTTP(resp, req)
-
- c.Check(logbuf.String(), check.Matches, `(?ms).*msg="File upload".*`)
- c.Check(logbuf.String(), check.Not(check.Matches), `(?ms).*level=error.*`)
}
-func (s *IntegrationSuite) TestDownloadPermission(c *check.C) {
+func (s *IntegrationSuite) TestDownloadLoggingPermission(c *check.C) {
config := newConfig(s.ArvConfig)
h := handler{Config: config}
u := mustParseURL("http://" + arvadostest.FooCollection + ".keep-web.example/foo")
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
@@ -1264,22 +1259,8 @@ func (s *IntegrationSuite) TestDownloadPermission(c *check.C) {
"Authorization": {"Bearer " + arvadostest.AdminToken},
},
}
-
- var logbuf bytes.Buffer
- logger := logrus.New()
- logger.Out = &logbuf
- resp := httptest.NewRecorder()
- req = req.WithContext(ctxlog.Context(context.Background(), logger))
- h.ServeHTTP(resp, req)
-
- if adminperm {
- c.Check(resp.Result().StatusCode, check.Equals, http.StatusOK)
- c.Check(logbuf.String(), check.Matches, `(?ms).*msg="File download".*`)
- c.Check(logbuf.String(), check.Not(check.Matches), `(?ms).*level=error.*`)
- } else {
- c.Check(resp.Result().StatusCode, check.Equals, http.StatusForbidden)
- c.Check(logbuf.String(), check.Equals, "")
- }
+ s.checkUploadDownloadRequest(c, &h, req, http.StatusOK, "download", adminperm,
+ arvadostest.AdminUserUUID, arvadostest.FooCollection, "foo")
// Test user permission
req = &http.Request{
@@ -1291,27 +1272,13 @@ func (s *IntegrationSuite) TestDownloadPermission(c *check.C) {
"Authorization": {"Bearer " + arvadostest.ActiveToken},
},
}
-
- logbuf = bytes.Buffer{}
- logger = logrus.New()
- logger.Out = &logbuf
- resp = httptest.NewRecorder()
- req = req.WithContext(ctxlog.Context(context.Background(), logger))
- h.ServeHTTP(resp, req)
-
- if userperm {
- c.Check(resp.Result().StatusCode, check.Equals, http.StatusOK)
- c.Check(logbuf.String(), check.Matches, `(?ms).*msg="File download".*`)
- c.Check(logbuf.String(), check.Not(check.Matches), `(?ms).*level=error.*`)
- } else {
- c.Check(resp.Result().StatusCode, check.Equals, http.StatusForbidden)
- c.Check(logbuf.String(), check.Equals, "")
- }
+ s.checkUploadDownloadRequest(c, &h, req, http.StatusOK, "download", userperm,
+ arvadostest.ActiveUserUUID, arvadostest.FooCollection, "foo")
}
}
}
-func (s *IntegrationSuite) TestUploadPermission(c *check.C) {
+func (s *IntegrationSuite) TestUploadLoggingPermission(c *check.C) {
defer func() {
client := s.testServer.Config.Client
client.AuthToken = arvadostest.AdminToken
@@ -1320,11 +1287,10 @@ func (s *IntegrationSuite) TestUploadPermission(c *check.C) {
config := newConfig(s.ArvConfig)
h := handler{Config: config}
- u := mustParseURL("http://" + arvadostest.FooCollection + ".keep-web.example/foo")
+ 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
@@ -1339,22 +1305,8 @@ func (s *IntegrationSuite) TestUploadPermission(c *check.C) {
},
Body: io.NopCloser(bytes.NewReader([]byte("bar"))),
}
-
- var logbuf bytes.Buffer
- logger := logrus.New()
- logger.Out = &logbuf
- resp := httptest.NewRecorder()
- req = req.WithContext(ctxlog.Context(context.Background(), logger))
- h.ServeHTTP(resp, req)
-
- if adminperm {
- c.Check(resp.Result().StatusCode, check.Equals, http.StatusCreated)
- c.Check(logbuf.String(), check.Matches, `(?ms).*msg="File upload".*`)
- c.Check(logbuf.String(), check.Not(check.Matches), `(?ms).*level=error.*`)
- } else {
- c.Check(resp.Result().StatusCode, check.Equals, http.StatusForbidden)
- c.Check(logbuf.String(), check.Equals, "")
- }
+ s.checkUploadDownloadRequest(c, &h, req, http.StatusCreated, "upload", adminperm,
+ arvadostest.AdminUserUUID, arvadostest.FooCollection, "bar")
// Test user permission
req = &http.Request{
@@ -1367,22 +1319,8 @@ func (s *IntegrationSuite) TestUploadPermission(c *check.C) {
},
Body: io.NopCloser(bytes.NewReader([]byte("bar"))),
}
-
- logbuf = bytes.Buffer{}
- logger = logrus.New()
- logger.Out = &logbuf
- resp = httptest.NewRecorder()
- req = req.WithContext(ctxlog.Context(context.Background(), logger))
- h.ServeHTTP(resp, req)
-
- if userperm {
- c.Check(resp.Result().StatusCode, check.Equals, http.StatusCreated)
- c.Check(logbuf.String(), check.Matches, `(?ms).*msg="File upload".*`)
- c.Check(logbuf.String(), check.Not(check.Matches), `(?ms).*level=error.*`)
- } else {
- c.Check(resp.Result().StatusCode, check.Equals, http.StatusForbidden)
- c.Check(logbuf.String(), check.Equals, "")
- }
+ s.checkUploadDownloadRequest(c, &h, req, http.StatusCreated, "upload", userperm,
+ arvadostest.ActiveUserUUID, arvadostest.FooCollection, "bar")
}
}
}
commit 59107add9b9d01d98cca49933c90f4e320d47fef
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Tue Jun 15 14:54:16 2021 -0400
17464: Reset database after upload tests
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/services/keep-web/handler_test.go b/services/keep-web/handler_test.go
index e9f2eaaa3..ba416d7cd 100644
--- a/services/keep-web/handler_test.go
+++ b/services/keep-web/handler_test.go
@@ -93,8 +93,9 @@ func (s *UnitSuite) TestEmptyResponse(c *check.C) {
// If we return no content because the client sent an
// If-Modified-Since header, our response should be
- // 304, and we should not emit a log message.
- {true, true, http.StatusNotModified, ``},
+ // 304. We still expect a "File download" log since it
+ // counts as a file access for auditing.
+ {true, true, http.StatusNotModified, `(?ms).*msg="File download".*`},
} {
c.Logf("trial: %+v", trial)
arvadostest.StartKeep(2, true)
@@ -1214,6 +1215,7 @@ func (s *IntegrationSuite) TestDownloadLogging(c *check.C) {
func (s *IntegrationSuite) TestUploadLogging(c *check.C) {
defer func() {
client := s.testServer.Config.Client
+ client.AuthToken = arvadostest.AdminToken
client.RequestAndDecode(nil, "POST", "database/reset", nil, nil)
}()
@@ -1312,6 +1314,7 @@ func (s *IntegrationSuite) TestDownloadPermission(c *check.C) {
func (s *IntegrationSuite) TestUploadPermission(c *check.C) {
defer func() {
client := s.testServer.Config.Client
+ client.AuthToken = arvadostest.AdminToken
client.RequestAndDecode(nil, "POST", "database/reset", nil, nil)
}()
commit 23f6c01aaf4a3f501e908a689f128e77fc9f23aa
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Mon Jun 14 17:30:59 2021 -0400
17464: Permission/logging testing WIP
The upload tests are messing up the other tests by changing the
contents of the collection, still need to fix it.
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/services/keep-web/handler_test.go b/services/keep-web/handler_test.go
index b8c7443f0..e9f2eaaa3 100644
--- a/services/keep-web/handler_test.go
+++ b/services/keep-web/handler_test.go
@@ -9,6 +9,7 @@ import (
"context"
"fmt"
"html"
+ "io"
"io/ioutil"
"net/http"
"net/http/httptest"
@@ -1178,27 +1179,6 @@ func (s *IntegrationSuite) TestCacheWriteCollectionSamePDH(c *check.C) {
checkWithID(colls[0].UUID, http.StatusOK)
}
-// func (s *IntegrationSuite) TestUploadDownloadLogging(c *check.C) {
-// u := mustParseURL("http://" + arvadostest.FooCollection + ".keep-web.example/foo")
-// req := &http.Request{
-// Method: "GET",
-// Host: u.Host,
-// URL: u,
-// RequestURI: u.RequestURI(),
-// Header: http.Header{
-// "Authorization": {"Bearer " + arvadostest.ActiveToken},
-// },
-// }
-
-// var logbuf bytes.Buffer
-// logger := logrus.New()
-// logger.Out = &logbuf
-// req = req.WithContext(ctxlog.Context(context.Background(), logger))
-// s.doReq(req)
-
-// c.Check(logbuf.String(), check.Matches, `Download file*`)
-// }
-
func copyHeader(h http.Header) http.Header {
hc := http.Header{}
for k, v := range h {
@@ -1206,3 +1186,200 @@ func copyHeader(h http.Header) http.Header {
}
return hc
}
+
+func (s *IntegrationSuite) TestDownloadLogging(c *check.C) {
+ h := handler{Config: newConfig(s.ArvConfig)}
+ u := mustParseURL("http://" + arvadostest.FooCollection + ".keep-web.example/foo")
+ req := &http.Request{
+ Method: "GET",
+ Host: u.Host,
+ URL: u,
+ RequestURI: u.RequestURI(),
+ Header: http.Header{
+ "Authorization": {"Bearer " + arvadostest.ActiveToken},
+ },
+ }
+
+ var logbuf bytes.Buffer
+ logger := logrus.New()
+ logger.Out = &logbuf
+ resp := httptest.NewRecorder()
+ req = req.WithContext(ctxlog.Context(context.Background(), logger))
+ h.ServeHTTP(resp, req)
+
+ c.Check(logbuf.String(), check.Matches, `(?ms).*msg="File download".*`)
+ c.Check(logbuf.String(), check.Not(check.Matches), `(?ms).*level=error.*`)
+}
+
+func (s *IntegrationSuite) TestUploadLogging(c *check.C) {
+ defer func() {
+ client := s.testServer.Config.Client
+ client.RequestAndDecode(nil, "POST", "database/reset", nil, nil)
+ }()
+
+ h := handler{Config: newConfig(s.ArvConfig)}
+ u := mustParseURL("http://" + arvadostest.FooCollection + ".keep-web.example/bar")
+ req := &http.Request{
+ Method: "PUT",
+ Host: u.Host,
+ URL: u,
+ RequestURI: u.RequestURI(),
+ Header: http.Header{
+ "Authorization": {"Bearer " + arvadostest.ActiveToken},
+ },
+ Body: io.NopCloser(bytes.NewReader([]byte("bar"))),
+ }
+
+ var logbuf bytes.Buffer
+ logger := logrus.New()
+ logger.Out = &logbuf
+ resp := httptest.NewRecorder()
+ req = req.WithContext(ctxlog.Context(context.Background(), logger))
+ h.ServeHTTP(resp, req)
+
+ c.Check(logbuf.String(), check.Matches, `(?ms).*msg="File upload".*`)
+ c.Check(logbuf.String(), check.Not(check.Matches), `(?ms).*level=error.*`)
+}
+
+func (s *IntegrationSuite) TestDownloadPermission(c *check.C) {
+ config := newConfig(s.ArvConfig)
+ h := handler{Config: config}
+ u := mustParseURL("http://" + arvadostest.FooCollection + ".keep-web.example/foo")
+
+ 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
+
+ // Test admin permission
+ req := &http.Request{
+ Method: "GET",
+ Host: u.Host,
+ URL: u,
+ RequestURI: u.RequestURI(),
+ Header: http.Header{
+ "Authorization": {"Bearer " + arvadostest.AdminToken},
+ },
+ }
+
+ var logbuf bytes.Buffer
+ logger := logrus.New()
+ logger.Out = &logbuf
+ resp := httptest.NewRecorder()
+ req = req.WithContext(ctxlog.Context(context.Background(), logger))
+ h.ServeHTTP(resp, req)
+
+ if adminperm {
+ c.Check(resp.Result().StatusCode, check.Equals, http.StatusOK)
+ c.Check(logbuf.String(), check.Matches, `(?ms).*msg="File download".*`)
+ c.Check(logbuf.String(), check.Not(check.Matches), `(?ms).*level=error.*`)
+ } else {
+ c.Check(resp.Result().StatusCode, check.Equals, http.StatusForbidden)
+ c.Check(logbuf.String(), check.Equals, "")
+ }
+
+ // Test user permission
+ req = &http.Request{
+ Method: "GET",
+ Host: u.Host,
+ URL: u,
+ RequestURI: u.RequestURI(),
+ Header: http.Header{
+ "Authorization": {"Bearer " + arvadostest.ActiveToken},
+ },
+ }
+
+ logbuf = bytes.Buffer{}
+ logger = logrus.New()
+ logger.Out = &logbuf
+ resp = httptest.NewRecorder()
+ req = req.WithContext(ctxlog.Context(context.Background(), logger))
+ h.ServeHTTP(resp, req)
+
+ if userperm {
+ c.Check(resp.Result().StatusCode, check.Equals, http.StatusOK)
+ c.Check(logbuf.String(), check.Matches, `(?ms).*msg="File download".*`)
+ c.Check(logbuf.String(), check.Not(check.Matches), `(?ms).*level=error.*`)
+ } else {
+ c.Check(resp.Result().StatusCode, check.Equals, http.StatusForbidden)
+ c.Check(logbuf.String(), check.Equals, "")
+ }
+ }
+ }
+}
+
+func (s *IntegrationSuite) TestUploadPermission(c *check.C) {
+ defer func() {
+ client := s.testServer.Config.Client
+ client.RequestAndDecode(nil, "POST", "database/reset", nil, nil)
+ }()
+
+ config := newConfig(s.ArvConfig)
+ h := handler{Config: config}
+ u := mustParseURL("http://" + arvadostest.FooCollection + ".keep-web.example/foo")
+
+ 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
+
+ // Test admin permission
+ req := &http.Request{
+ Method: "PUT",
+ Host: u.Host,
+ URL: u,
+ RequestURI: u.RequestURI(),
+ Header: http.Header{
+ "Authorization": {"Bearer " + arvadostest.AdminToken},
+ },
+ Body: io.NopCloser(bytes.NewReader([]byte("bar"))),
+ }
+
+ var logbuf bytes.Buffer
+ logger := logrus.New()
+ logger.Out = &logbuf
+ resp := httptest.NewRecorder()
+ req = req.WithContext(ctxlog.Context(context.Background(), logger))
+ h.ServeHTTP(resp, req)
+
+ if adminperm {
+ c.Check(resp.Result().StatusCode, check.Equals, http.StatusCreated)
+ c.Check(logbuf.String(), check.Matches, `(?ms).*msg="File upload".*`)
+ c.Check(logbuf.String(), check.Not(check.Matches), `(?ms).*level=error.*`)
+ } else {
+ c.Check(resp.Result().StatusCode, check.Equals, http.StatusForbidden)
+ c.Check(logbuf.String(), check.Equals, "")
+ }
+
+ // Test user permission
+ req = &http.Request{
+ Method: "PUT",
+ Host: u.Host,
+ URL: u,
+ RequestURI: u.RequestURI(),
+ Header: http.Header{
+ "Authorization": {"Bearer " + arvadostest.ActiveToken},
+ },
+ Body: io.NopCloser(bytes.NewReader([]byte("bar"))),
+ }
+
+ logbuf = bytes.Buffer{}
+ logger = logrus.New()
+ logger.Out = &logbuf
+ resp = httptest.NewRecorder()
+ req = req.WithContext(ctxlog.Context(context.Background(), logger))
+ h.ServeHTTP(resp, req)
+
+ if userperm {
+ c.Check(resp.Result().StatusCode, check.Equals, http.StatusCreated)
+ c.Check(logbuf.String(), check.Matches, `(?ms).*msg="File upload".*`)
+ c.Check(logbuf.String(), check.Not(check.Matches), `(?ms).*level=error.*`)
+ } else {
+ c.Check(resp.Result().StatusCode, check.Equals, http.StatusForbidden)
+ c.Check(logbuf.String(), check.Equals, "")
+ }
+ }
+ }
+}
diff --git a/services/keep-web/server_test.go b/services/keep-web/server_test.go
index 5c68eb424..a65a48892 100644
--- a/services/keep-web/server_test.go
+++ b/services/keep-web/server_test.go
@@ -34,6 +34,7 @@ var _ = check.Suite(&IntegrationSuite{})
// IntegrationSuite tests need an API server and a keep-web server
type IntegrationSuite struct {
testServer *server
+ ArvConfig *arvados.Config
}
func (s *IntegrationSuite) TestNoToken(c *check.C) {
@@ -389,7 +390,7 @@ func (s *IntegrationSuite) TestMetrics(c *check.C) {
c.Check(summaries["request_duration_seconds/get/404"].SampleCount, check.Equals, "1")
c.Check(summaries["time_to_status_seconds/get/404"].SampleCount, check.Equals, "1")
c.Check(counters["arvados_keepweb_collectioncache_requests//"].Value, check.Equals, int64(2))
- c.Check(counters["arvados_keepweb_collectioncache_api_calls//"].Value, check.Equals, int64(1))
+ c.Check(counters["arvados_keepweb_collectioncache_api_calls//"].Value, check.Equals, int64(2))
c.Check(counters["arvados_keepweb_collectioncache_hits//"].Value, check.Equals, int64(1))
c.Check(counters["arvados_keepweb_collectioncache_pdh_hits//"].Value, check.Equals, int64(1))
c.Check(counters["arvados_keepweb_collectioncache_permission_hits//"].Value, check.Equals, int64(1))
@@ -446,6 +447,7 @@ func (s *IntegrationSuite) SetUpTest(c *check.C) {
cfg.cluster.ManagementToken = arvadostest.ManagementToken
cfg.cluster.SystemRootToken = arvadostest.SystemRootToken
cfg.cluster.Users.AnonymousUserToken = arvadostest.AnonymousToken
+ s.ArvConfig = arvCfg
s.testServer = &server{Config: cfg}
err = s.testServer.Start(ctxlog.TestLogger(c))
c.Assert(err, check.Equals, nil)
commit 3a8485be4eeb09f082338bc3dabe11a3702e538c
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Fri Jun 11 17:19:36 2021 -0400
17464: Fix shadowed "sess" variable
Testing WIP
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/services/keep-web/cache.go b/services/keep-web/cache.go
index 350e95ce8..a52af8048 100644
--- a/services/keep-web/cache.go
+++ b/services/keep-web/cache.go
@@ -225,7 +225,7 @@ func (c *cache) GetSession(token string) (arvados.CustomFileSystem, *cachedSessi
expired := false
if sess == nil {
c.metrics.sessionMisses.Inc()
- sess := &cachedSession{
+ sess = &cachedSession{
expire: now.Add(c.config.TTL.Duration()),
}
var err error
diff --git a/services/keep-web/handler_test.go b/services/keep-web/handler_test.go
index 8715ab24f..b8c7443f0 100644
--- a/services/keep-web/handler_test.go
+++ b/services/keep-web/handler_test.go
@@ -1178,6 +1178,27 @@ func (s *IntegrationSuite) TestCacheWriteCollectionSamePDH(c *check.C) {
checkWithID(colls[0].UUID, http.StatusOK)
}
+// func (s *IntegrationSuite) TestUploadDownloadLogging(c *check.C) {
+// u := mustParseURL("http://" + arvadostest.FooCollection + ".keep-web.example/foo")
+// req := &http.Request{
+// Method: "GET",
+// Host: u.Host,
+// URL: u,
+// RequestURI: u.RequestURI(),
+// Header: http.Header{
+// "Authorization": {"Bearer " + arvadostest.ActiveToken},
+// },
+// }
+
+// var logbuf bytes.Buffer
+// logger := logrus.New()
+// logger.Out = &logbuf
+// req = req.WithContext(ctxlog.Context(context.Background(), logger))
+// s.doReq(req)
+
+// c.Check(logbuf.String(), check.Matches, `Download file*`)
+// }
+
func copyHeader(h http.Header) http.Header {
hc := http.Header{}
for k, v := range h {
commit ce4ece370e62774f1e3ef797c714529603943f41
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Fri Jun 11 15:26:05 2021 -0400
17464: Log collection uuid for FileSystem requests
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/sdk/go/arvados/fs_collection.go b/sdk/go/arvados/fs_collection.go
index 22e2b31d5..b743ab368 100644
--- a/sdk/go/arvados/fs_collection.go
+++ b/sdk/go/arvados/fs_collection.go
@@ -674,6 +674,7 @@ func (dn *dirnode) Child(name string, replace func(inode) (inode, error)) (inode
if err != nil {
return nil, err
}
+ coll.UUID = dn.fs.uuid
data, err := json.Marshal(&coll)
if err == nil {
data = append(data, '\n')
diff --git a/services/keep-web/handler.go b/services/keep-web/handler.go
index ba9114664..43ce57904 100644
--- a/services/keep-web/handler.go
+++ b/services/keep-web/handler.go
@@ -491,7 +491,7 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
http.Error(w, "Not permitted", http.StatusForbidden)
return
}
- h.LogUploadOrDownload(r, sess.arvadosclient, collection, tokenUser)
+ h.LogUploadOrDownload(r, sess.arvadosclient, nil, strings.Join(targetPath, "/"), collection, tokenUser)
if webdavMethod[r.Method] {
if writeMethod[r.Method] {
@@ -623,7 +623,7 @@ func (h *handler) serveSiteFS(w http.ResponseWriter, r *http.Request, tokens []s
http.Error(w, "Not permitted", http.StatusForbidden)
return
}
- h.LogUploadOrDownload(r, sess.arvadosclient, nil, tokenUser)
+ h.LogUploadOrDownload(r, sess.arvadosclient, fs, r.URL.Path, nil, tokenUser)
if r.Method == "GET" {
_, basename := filepath.Split(r.URL.Path)
@@ -882,17 +882,29 @@ func (h *handler) UserPermittedToUploadOrDownload(method string, tokenUser *arva
return true
}
-func (h *handler) LogUploadOrDownload(r *http.Request, client *arvadosclient.ArvadosClient, collection *arvados.Collection, user *arvados.User) {
+func (h *handler) LogUploadOrDownload(
+ r *http.Request,
+ client *arvadosclient.ArvadosClient,
+ fs arvados.CustomFileSystem,
+ filepath string,
+ collection *arvados.Collection,
+ user *arvados.User) {
+
log := ctxlog.FromContext(r.Context())
props := make(map[string]string)
props["reqPath"] = r.URL.Path
if user != nil {
log = log.WithField("user_uuid", user.UUID).
- WithField("full_name", user.FullName)
+ WithField("user_full_name", user.FullName)
+ }
+ if collection == nil && fs != nil {
+ collection, filepath = h.DetermineCollection(fs, filepath)
}
if collection != nil {
- log = log.WithField("collection_uuid", collection.UUID)
+ log = log.WithField("collection_uuid", collection.UUID).
+ WithField("collection_file_path", filepath)
props["collection_uuid"] = collection.UUID
+ props["collection_file_path"] = filepath
}
if r.Method == "PUT" || r.Method == "POST" {
log.Info("File upload")
@@ -904,7 +916,7 @@ func (h *handler) LogUploadOrDownload(r *http.Request, client *arvadosclient.Arv
client.Create("logs", lr, nil)
}()
} else if r.Method == "GET" {
- if collection != nil {
+ if collection != nil && collection.PortableDataHash != "" {
log = log.WithField("portable_data_hash", collection.PortableDataHash)
props["portable_data_hash"] = collection.PortableDataHash
}
@@ -918,3 +930,24 @@ func (h *handler) LogUploadOrDownload(r *http.Request, client *arvadosclient.Arv
}()
}
}
+
+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-- {
+ 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 {
+ return nil, ""
+ }
+ return &collection, strings.Join(segments[i:], "/")
+ }
+ f.Close()
+ }
+ return nil, ""
+}
diff --git a/services/keep-web/s3.go b/services/keep-web/s3.go
index 6a0c94331..357c42ae6 100644
--- a/services/keep-web/s3.go
+++ b/services/keep-web/s3.go
@@ -410,7 +410,7 @@ func (h *handler) serveS3(w http.ResponseWriter, r *http.Request) bool {
http.Error(w, "Not permitted", http.StatusForbidden)
return true
}
- h.LogUploadOrDownload(r, arvclient, nil, tokenUser)
+ h.LogUploadOrDownload(r, arvclient, fs, fspath, nil, tokenUser)
// shallow copy r, and change URL path
r := *r
@@ -501,7 +501,7 @@ func (h *handler) serveS3(w http.ResponseWriter, r *http.Request) bool {
http.Error(w, "Not permitted", http.StatusForbidden)
return true
}
- h.LogUploadOrDownload(r, arvclient, nil, tokenUser)
+ h.LogUploadOrDownload(r, arvclient, fs, fspath, nil, tokenUser)
_, err = io.Copy(f, r.Body)
if err != nil {
commit 834ba51d22fa93297e66c60c3eb51cc1cf05fccc
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Thu Jun 10 16:50:56 2021 -0400
17464: Add upload/download permission checks and logging
Adds extra log line to normal logging that includes user uuid and full
name.
Also posts an event to the logs table.
Adds permission checks to but these haven't been tested yet.
Still need to add testing.
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/services/keep-web/cache.go b/services/keep-web/cache.go
index 9bdecdca1..350e95ce8 100644
--- a/services/keep-web/cache.go
+++ b/services/keep-web/cache.go
@@ -131,8 +131,12 @@ type cachedPermission struct {
}
type cachedSession struct {
- expire time.Time
- fs atomic.Value
+ expire time.Time
+ fs atomic.Value
+ client *arvados.Client
+ arvadosclient *arvadosclient.ArvadosClient
+ keepclient *keepclient.KeepClient
+ user atomic.Value
}
func (c *cache) setup() {
@@ -213,7 +217,7 @@ func (c *cache) ResetSession(token string) {
// Get a long-lived CustomFileSystem suitable for doing a read operation
// with the given token.
-func (c *cache) GetSession(token string) (arvados.CustomFileSystem, error) {
+func (c *cache) GetSession(token string) (arvados.CustomFileSystem, *cachedSession, error) {
c.setupOnce.Do(c.setup)
now := time.Now()
ent, _ := c.sessions.Get(token)
@@ -221,9 +225,20 @@ func (c *cache) GetSession(token string) (arvados.CustomFileSystem, error) {
expired := false
if sess == nil {
c.metrics.sessionMisses.Inc()
- sess = &cachedSession{
+ sess := &cachedSession{
expire: now.Add(c.config.TTL.Duration()),
}
+ var err error
+ sess.client, err = arvados.NewClientFromConfig(c.cluster)
+ if err != nil {
+ return nil, nil, err
+ }
+ sess.client.AuthToken = token
+ sess.arvadosclient, err = arvadosclient.New(sess.client)
+ if err != nil {
+ return nil, nil, err
+ }
+ sess.keepclient = keepclient.New(sess.arvadosclient)
c.sessions.Add(token, sess)
} else if sess.expire.Before(now) {
c.metrics.sessionMisses.Inc()
@@ -234,22 +249,12 @@ func (c *cache) GetSession(token string) (arvados.CustomFileSystem, error) {
go c.pruneSessions()
fs, _ := sess.fs.Load().(arvados.CustomFileSystem)
if fs != nil && !expired {
- return fs, nil
+ return fs, sess, nil
}
- ac, err := arvados.NewClientFromConfig(c.cluster)
- if err != nil {
- return nil, err
- }
- ac.AuthToken = token
- arv, err := arvadosclient.New(ac)
- if err != nil {
- return nil, err
- }
- kc := keepclient.New(arv)
- fs = ac.SiteFileSystem(kc)
+ fs = sess.client.SiteFileSystem(sess.keepclient)
fs.ForwardSlashNameSubstitution(c.cluster.Collections.ForwardSlashNameSubstitution)
sess.fs.Store(fs)
- return fs, nil
+ return fs, sess, nil
}
// Remove all expired session cache entries, then remove more entries
@@ -464,3 +469,35 @@ func (c *cache) lookupCollection(key string) *arvados.Collection {
c.metrics.collectionHits.Inc()
return ent.collection
}
+
+func (c *cache) GetTokenUser(token string) (*arvados.User, error) {
+ // Get and cache user record associated with this
+ // token. We need to know their UUID for logging, and
+ // whether they are an admin or not for certain
+ // permission checks.
+
+ // Get/create session entry
+ _, sess, err := c.GetSession(token)
+ if err != nil {
+ return nil, err
+ }
+
+ // See if the user is already set, and if so, return it
+ user, _ := sess.user.Load().(*arvados.User)
+ if user != nil {
+ return user, nil
+ }
+
+ // Fetch the user record
+ c.metrics.apiCalls.Inc()
+ var current arvados.User
+
+ err = sess.client.RequestAndDecode(¤t, "GET", "/arvados/v1/users/current", nil, nil)
+ if err != nil {
+ return nil, err
+ }
+
+ // Stash the user record for next time
+ sess.user.Store(¤t)
+ return ¤t, nil
+}
diff --git a/services/keep-web/handler.go b/services/keep-web/handler.go
index 81925421d..ba9114664 100644
--- a/services/keep-web/handler.go
+++ b/services/keep-web/handler.go
@@ -398,6 +398,7 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
defer h.clientPool.Put(arv)
var collection *arvados.Collection
+ var tokenUser *arvados.User
tokenResult := make(map[string]int)
for _, arv.ApiToken = range tokens {
var err error
@@ -483,6 +484,15 @@ func (h *handler) ServeHTTP(wOrig http.ResponseWriter, r *http.Request) {
return
}
+ // 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) {
+ http.Error(w, "Not permitted", http.StatusForbidden)
+ return
+ }
+ h.LogUploadOrDownload(r, sess.arvadosclient, collection, tokenUser)
+
if webdavMethod[r.Method] {
if writeMethod[r.Method] {
// Save the collection only if/when all
@@ -583,7 +593,8 @@ func (h *handler) serveSiteFS(w http.ResponseWriter, r *http.Request, tokens []s
http.Error(w, errReadOnly.Error(), http.StatusMethodNotAllowed)
return
}
- fs, err := h.Config.Cache.GetSession(tokens[0])
+
+ fs, sess, err := h.Config.Cache.GetSession(tokens[0])
if err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
return
@@ -606,6 +617,14 @@ func (h *handler) serveSiteFS(w http.ResponseWriter, r *http.Request, tokens []s
}
return
}
+
+ tokenUser, err := h.Config.Cache.GetTokenUser(tokens[0])
+ if !h.UserPermittedToUploadOrDownload(r.Method, tokenUser) {
+ http.Error(w, "Not permitted", http.StatusForbidden)
+ return
+ }
+ h.LogUploadOrDownload(r, sess.arvadosclient, nil, tokenUser)
+
if r.Method == "GET" {
_, basename := filepath.Split(r.URL.Path)
applyContentDispositionHdr(w, r, basename, attachment)
@@ -836,3 +855,66 @@ func (h *handler) seeOtherWithCookie(w http.ResponseWriter, r *http.Request, loc
io.WriteString(w, html.EscapeString(redir))
io.WriteString(w, `">Continue</A>`)
}
+
+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
+ } else {
+ permitUpload = h.Config.cluster.Collections.KeepWebPermission.User.Upload
+ permitDownload = h.Config.cluster.Collections.KeepWebPermission.User.Download
+ }
+ if (method == "PUT" || method == "POST") && !permitUpload {
+ // Disallow operations that upload new files.
+ // Permit webdav operations that move existing files around.
+ return false
+ } else if method == "GET" && !permitDownload {
+ // Disallow downloading file contents.
+ // Permit webdav operations like PROPFIND that retrieve metadata
+ // but not file contents.
+ return false
+ }
+ return true
+}
+
+func (h *handler) LogUploadOrDownload(r *http.Request, client *arvadosclient.ArvadosClient, collection *arvados.Collection, user *arvados.User) {
+ log := ctxlog.FromContext(r.Context())
+ props := make(map[string]string)
+ props["reqPath"] = r.URL.Path
+ if user != nil {
+ log = log.WithField("user_uuid", user.UUID).
+ WithField("full_name", user.FullName)
+ }
+ if collection != nil {
+ log = log.WithField("collection_uuid", collection.UUID)
+ props["collection_uuid"] = collection.UUID
+ }
+ 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)
+ }()
+ } else if r.Method == "GET" {
+ if collection != nil {
+ 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)
+ }()
+ }
+}
diff --git a/services/keep-web/s3.go b/services/keep-web/s3.go
index 6ea9bf9f7..6a0c94331 100644
--- a/services/keep-web/s3.go
+++ b/services/keep-web/s3.go
@@ -24,7 +24,9 @@ import (
"time"
"git.arvados.org/arvados.git/sdk/go/arvados"
+ "git.arvados.org/arvados.git/sdk/go/arvadosclient"
"git.arvados.org/arvados.git/sdk/go/ctxlog"
+ "git.arvados.org/arvados.git/sdk/go/keepclient"
"github.com/AdRoll/goamz/s3"
)
@@ -309,19 +311,25 @@ func (h *handler) serveS3(w http.ResponseWriter, r *http.Request) bool {
var err error
var fs arvados.CustomFileSystem
+ var arvclient *arvadosclient.ArvadosClient
if r.Method == http.MethodGet || r.Method == http.MethodHead {
// Use a single session (cached FileSystem) across
// multiple read requests.
- fs, err = h.Config.Cache.GetSession(token)
+ var sess *cachedSession
+ fs, sess, err = h.Config.Cache.GetSession(token)
if err != nil {
s3ErrorResponse(w, InternalError, err.Error(), r.URL.Path, http.StatusInternalServerError)
return true
}
+ arvclient = sess.arvadosclient
} else {
// Create a FileSystem for this request, to avoid
// exposing incomplete write operations to concurrent
// requests.
- _, kc, client, release, err := h.getClients(r.Header.Get("X-Request-Id"), token)
+ var kc *keepclient.KeepClient
+ var release func()
+ var client *arvados.Client
+ arvclient, kc, client, release, err = h.getClients(r.Header.Get("X-Request-Id"), token)
if err != nil {
s3ErrorResponse(w, InternalError, err.Error(), r.URL.Path, http.StatusInternalServerError)
return true
@@ -396,6 +404,14 @@ func (h *handler) serveS3(w http.ResponseWriter, r *http.Request) bool {
s3ErrorResponse(w, NoSuchKey, "The specified key does not exist.", r.URL.Path, http.StatusNotFound)
return true
}
+
+ tokenUser, err := h.Config.Cache.GetTokenUser(token)
+ if !h.UserPermittedToUploadOrDownload(r.Method, tokenUser) {
+ http.Error(w, "Not permitted", http.StatusForbidden)
+ return true
+ }
+ h.LogUploadOrDownload(r, arvclient, nil, tokenUser)
+
// shallow copy r, and change URL path
r := *r
r.URL.Path = fspath
@@ -479,6 +495,14 @@ func (h *handler) serveS3(w http.ResponseWriter, r *http.Request) bool {
return true
}
defer f.Close()
+
+ tokenUser, err := h.Config.Cache.GetTokenUser(token)
+ if !h.UserPermittedToUploadOrDownload(r.Method, tokenUser) {
+ http.Error(w, "Not permitted", http.StatusForbidden)
+ return true
+ }
+ h.LogUploadOrDownload(r, arvclient, nil, tokenUser)
+
_, err = io.Copy(f, r.Body)
if err != nil {
err = fmt.Errorf("write to %q failed: %w", r.URL.Path, err)
commit 69b64f2fe8ccd2b92d1aa55b9fb39b03a342468e
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Mon Jun 7 15:42:32 2021 -0400
17464: Fix config entry
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index e50dd8a61..537db6762 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -558,7 +558,7 @@ Clusters:
# able to download or upload data files using the
# upload/download features for Workbench, WebDAV and S3 API
# support.
- KeepWebPermisison:
+ KeepWebPermission:
User:
Download: true
Upload: true
diff --git a/lib/config/export.go b/lib/config/export.go
index 23d0b6bff..fb1420aef 100644
--- a/lib/config/export.go
+++ b/lib/config/export.go
@@ -106,6 +106,8 @@ var whitelist = map[string]bool{
"Collections.TrashSweepInterval": false,
"Collections.TrustAllContent": false,
"Collections.WebDAVCache": false,
+ "Collections.KeepproxyPermission": false,
+ "Collections.KeepWebPermission": false,
"Containers": true,
"Containers.CloudVMs": false,
"Containers.CrunchRunArgumentsList": false,
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index d5e0f200b..e2ba3bc2c 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -560,6 +560,29 @@ Clusters:
# Persistent sessions.
MaxSessions: 100
+ # Selectively set permissions for regular users and admins to be
+ # able to download or upload data files using the
+ # upload/download features for Workbench, WebDAV and S3 API
+ # support.
+ KeepWebPermission:
+ User:
+ Download: true
+ Upload: true
+ Admin:
+ Download: true
+ Upload: true
+
+ # Selectively set permissions for regular users and admins to be
+ # able to download or upload blocks using arv-put and
+ # arv-get from outside the cluster.
+ KeepproxyPermission:
+ User:
+ Download: true
+ Upload: true
+ Admin:
+ Download: true
+ Upload: true
+
Login:
# One of the following mechanisms (SSO, Google, PAM, LDAP, or
# LoginCluster) should be enabled; see
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index 403d501b4..83a670832 100644
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -68,6 +68,16 @@ type WebDAVCacheConfig struct {
MaxSessions int
}
+type UploadDownloadPermission struct {
+ Upload bool
+ Download bool
+}
+
+type UploadDownloadRolePermissions struct {
+ User UploadDownloadPermission
+ Admin UploadDownloadPermission
+}
+
type Cluster struct {
ClusterID string `json:"-"`
ManagementToken string
@@ -130,6 +140,9 @@ type Cluster struct {
BalanceTimeout Duration
WebDAVCache WebDAVCacheConfig
+
+ KeepproxyPermission UploadDownloadRolePermissions
+ KeepWebPermission UploadDownloadRolePermissions
}
Git struct {
GitCommand string
commit de8448b3b546eb1eee8a45261954028e3ea22252
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Mon Jun 7 15:23:40 2021 -0400
17464: Start by writing the documentation page
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/doc/admin/restricting-upload-download.html.textile.liquid b/doc/admin/restricting-upload-download.html.textile.liquid
new file mode 100644
index 000000000..602cdfd61
--- /dev/null
+++ b/doc/admin/restricting-upload-download.html.textile.liquid
@@ -0,0 +1,127 @@
+---
+layout: default
+navsection: admin
+title: Restricting upload or download
+...
+
+{% comment %}
+Copyright (C) The Arvados Authors. All rights reserved.
+
+SPDX-License-Identifier: CC-BY-SA-3.0
+{% endcomment %}
+
+For some use cases, you may want to limit the ability of users to upload or download data from outside the cluster. (By "outside" we mean from networks other than the cluster's own private network). For example, this makes it possible to share restricted data sets with users so that they may run their own data analysis on the cluster, while preventing them from easily downloading the data set to their local workstation.
+
+This feature exists in addition to the existing Arvados permission system. Users can only download from collections they have @read@ access to, and can only upload to projects and collections they have @write@ access to.
+
+h2. Keep-web and Keepproxy Permissions
+
+There are two services involved in accessing data from outside the cluster.
+
+ at keeproxy@ makes it possible to use @arv-put@ and @arv-get at . It works in terms of individual 64 MiB keep blocks. It prints a log each time a user uploads or downloads an individual block.
+
+ at keep-web@ makes it possible to use Workbench, WebDAV and S3 API. It works in terms of individual files. It prints a log each time a user uploads or downloads a file, and also adds an entry into the API server @logs@ table.
+
+This distinction is important for auditing, the @keep-web@ records 'upload' and 'download' events on the API server that are included in the "User Activity Report":user-activity.html , whereas @keepprox@ only logs upload and download of individual blocks, which require a reverse lookup to determine the collection(s) and file(s) a block is associated with.
+
+You can set permissions for @keep-web@ and @keepproxy@, with separate policies for regular users and admin users.
+
+If the user attempts to upload or download from a service without permission, they will receive a @403 Forbidden@ response. This only applies to file content. Users can still see collection listings.
+
+The default policy allows anyone to upload or download.
+
+<pre>
+ Collections:
+ KeepWebPermisison:
+ User:
+ Download: true
+ Upload: true
+ Admin:
+ Download: true
+ Upload: true
+
+ KeepproxyPermission:
+ User:
+ Download: true
+ Upload: true
+ Admin:
+ Download: true
+ Upload: true
+</pre>
+
+h2. Shell node and container permissions
+
+Be aware that even when upload and download from outside the network is not allowed, a user who has access to a shell node or runs a container still has internal access to Keep. (This is necessary to be able to run workflows). From the shell node or container, a user could send data outside the network by some other method, although this requires more intent than accidentally clicking on a link and downloading a file. It is possible to set up a firewall to prevent shell and compute nodes from making connections to hosts outside the private network. Exactly how to configure this is out of scope for this page, as it depends on the specific network infrastructure of your cluster.
+
+h2. Choosing a policy
+
+These policies apply to only access from outside the cluster, using Workbench or Arvados CLI tools.
+
+h3. Audited downloads
+
+For ease of access auditing, this policy prevents downloads using @arv-get at . Downloads through @keep-web@ are permitted, but logged. Uploads with @arv-put@ are allowed.
+
+<pre>
+ Collections:
+ KeepWebPermisison:
+ User:
+ Download: true
+ Upload: true
+ Admin:
+ Download: true
+ Upload: true
+
+ KeepproxyPermission:
+ User:
+ Download: false
+ Upload: true
+ Admin:
+ Download: false
+ Upload: true
+</pre>
+
+h3. Disallow downloads by regular users
+
+This policy prevents regular users (non-admin) from downloading data. Uploading is allowed. This supports the case where restricted data sets are shared users so that they may run their own data analysis on the cluster, while preventing them from downloading the data set to their local workstation. Note that users won't be able to download the results of their analysis, either, requiring an admin in the loop or some other process to release results.
+
+<pre>
+ Collections:
+ KeepWebPermisison:
+ User:
+ Download: false
+ Upload: true
+ Admin:
+ Download: true
+ Upload: true
+
+ KeepproxyPermission:
+ User:
+ Download: false
+ Upload: true
+ Admin:
+ Download: true
+ Upload: true
+</pre>
+
+h3. Disallow uploads by regular users
+
+This policy is suitable for an installation where data is being shared with a group of users who are allowed to download the data, but not permitted to store their own data on the cluster.
+
+<pre>
+ Collections:
+ KeepWebPermisison:
+ User:
+ Download: true
+ Upload: false
+ Admin:
+ Download: true
+ Upload: true
+
+ KeepproxyPermission:
+ User:
+ Download: true
+ Upload: false
+ Admin:
+ Download: true
+ Upload: true
+</pre>
diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index f0794a7e5..e50dd8a61 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -554,6 +554,29 @@ Clusters:
# Persistent sessions.
MaxSessions: 100
+ # Selectively set permissions for regular users and admins to be
+ # able to download or upload data files using the
+ # upload/download features for Workbench, WebDAV and S3 API
+ # support.
+ KeepWebPermisison:
+ User:
+ Download: true
+ Upload: true
+ Admin:
+ Download: true
+ Upload: true
+
+ # Selectively set permissions for regular users and admins to be
+ # able to download or upload blocks using arv-put and
+ # arv-get from outside the cluster.
+ KeepproxyPermission:
+ User:
+ Download: true
+ Upload: true
+ Admin:
+ Download: true
+ Upload: true
+
Login:
# One of the following mechanisms (SSO, Google, PAM, LDAP, or
# LoginCluster) should be enabled; see
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list