[ARVADOS] updated: 2.1.0-890-gfa956f9af
Git user
git at public.arvados.org
Thu Jun 10 20:54:41 UTC 2021
Summary of changes:
lib/config/config.default.yml | 2 +-
lib/config/export.go | 2 +
lib/config/generated_config.go | 23 ++++++++++++
sdk/go/arvados/config.go | 13 +++++++
services/keep-web/cache.go | 71 ++++++++++++++++++++++++++---------
services/keep-web/handler.go | 84 +++++++++++++++++++++++++++++++++++++++++-
services/keep-web/s3.go | 28 +++++++++++++-
7 files changed, 202 insertions(+), 21 deletions(-)
via fa956f9afdf53aafaa5b8fab1bb923af03d6120c (commit)
via d3796adf4fca639b6741e1e0ddbe049a32b5e157 (commit)
from 3237f96388a0c0ca2ec299265c6e51eab55a2d2d (commit)
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 fa956f9afdf53aafaa5b8fab1bb923af03d6120c
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 f03ff01b8..db693741b 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"
)
@@ -285,19 +287,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
@@ -372,6 +380,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
@@ -455,6 +471,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 d3796adf4fca639b6741e1e0ddbe049a32b5e157
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 ec8152fdb..13761bf76 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -559,7 +559,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 fbee937b3..49d6e5ed2 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -561,6 +561,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
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list