[arvados] updated: 2.7.0-5356-g135bada0fe
git repository hosting
git at public.arvados.org
Fri Nov 17 18:30:47 UTC 2023
Summary of changes:
lib/controller/federation/conn.go | 61 +++++++++++++++++++++++++----
lib/controller/localdb/container_gateway.go | 2 +-
sdk/go/arvados/user.go | 4 +-
services/keep-balance/balance.go | 2 +-
services/keep-web/handler.go | 2 +-
services/keepproxy/keepproxy.go | 2 +-
6 files changed, 60 insertions(+), 13 deletions(-)
via 135bada0fe08de2b678ede684d43a155c4351ed3 (commit)
from e42bf8fbe66f822066e13c08b346005a52e1aa4a (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 135bada0fe08de2b678ede684d43a155c4351ed3
Author: Peter Amstutz <peter.amstutz at curii.com>
Date: Fri Nov 17 13:29:29 2023 -0500
20831: Revert making IsAdmin and IsInvited a pointer
Now uses API server revision and the requesting user to decide whether
IsAdmin and IsInvited have valid values or not.
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>
diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go
index 24a7b6f883..bd5a6d812e 100644
--- a/lib/controller/federation/conn.go
+++ b/lib/controller/federation/conn.go
@@ -652,7 +652,8 @@ var userAttrsCachedFromLoginCluster = map[string]bool{
func (conn *Conn) batchUpdateUsers(ctx context.Context,
options arvados.ListOptions,
- items []arvados.User) (err error) {
+ items []arvados.User,
+ includeAdminAndInvited bool) (err error) {
id := conn.cluster.Login.LoginCluster
logger := ctxlog.FromContext(ctx)
@@ -699,6 +700,12 @@ func (conn *Conn) batchUpdateUsers(ctx context.Context,
}
}
}
+ if !includeAdminAndInvited {
+ // make sure we don't send these fields.
+ delete(updates, "is_admin")
+ delete(updates, "is_invited")
+ }
+ fmt.Printf("updates %v", updates)
batchOpts.Updates[user.UUID] = updates
}
if len(batchOpts.Updates) > 0 {
@@ -711,13 +718,43 @@ func (conn *Conn) batchUpdateUsers(ctx context.Context,
return nil
}
+func (conn *Conn) includeAdminAndInvitedInBatchUpdate(ctx context.Context, be backend, updateUserUUID string) (bool, error) {
+ // API versions prior to 20231117 would only include the
+ // is_invited and is_admin fields if the current user is an
+ // admin, or is requesting their own user record. If those
+ // fields aren't actually valid then we don't want to
+ // send them in the batch update.
+ dd, err := be.DiscoveryDocument(ctx)
+ if dd.Revision >= "20231117" {
+ // newer version, fields are valid.
+ return true, nil
+ }
+ selfuser, err := be.UserGetCurrent(ctx, arvados.GetOptions{})
+ if err != nil {
+ // couldn't get our user record
+ return false, err
+ }
+ if selfuser.IsAdmin || selfuser.UUID == updateUserUUID {
+ // we are an admin, or the current user is the same as
+ // the user that we are updating.
+ return true, nil
+ }
+ // Better safe than sorry.
+ return false, nil
+}
+
func (conn *Conn) UserList(ctx context.Context, options arvados.ListOptions) (arvados.UserList, error) {
if id := conn.cluster.Login.LoginCluster; id != "" && id != conn.cluster.ClusterID && !options.BypassFederation {
- resp, err := conn.chooseBackend(id).UserList(ctx, options)
+ be := conn.chooseBackend(id)
+ resp, err := be.UserList(ctx, options)
if err != nil {
return resp, err
}
- err = conn.batchUpdateUsers(ctx, options, resp.Items)
+ includeAdminAndInvited, err := conn.includeAdminAndInvitedInBatchUpdate(ctx, be, "")
+ if err != nil {
+ return arvados.UserList{}, err
+ }
+ err = conn.batchUpdateUsers(ctx, options, resp.Items, includeAdminAndInvited)
if err != nil {
return arvados.UserList{}, err
}
@@ -734,13 +771,18 @@ func (conn *Conn) UserUpdate(ctx context.Context, options arvados.UpdateOptions)
if options.BypassFederation {
return conn.local.UserUpdate(ctx, options)
}
- resp, err := conn.chooseBackend(options.UUID).UserUpdate(ctx, options)
+ be := conn.chooseBackend(options.UUID)
+ resp, err := be.UserUpdate(ctx, options)
if err != nil {
return resp, err
}
if !strings.HasPrefix(options.UUID, conn.cluster.ClusterID) {
+ includeAdminAndInvited, err := conn.includeAdminAndInvitedInBatchUpdate(ctx, be, options.UUID)
+ if err != nil {
+ return arvados.User{}, err
+ }
// Copy the updated user record to the local cluster
- err = conn.batchUpdateUsers(ctx, arvados.ListOptions{}, []arvados.User{resp})
+ err = conn.batchUpdateUsers(ctx, arvados.ListOptions{}, []arvados.User{resp}, includeAdminAndInvited)
if err != nil {
return arvados.User{}, err
}
@@ -787,7 +829,8 @@ func (conn *Conn) UserUnsetup(ctx context.Context, options arvados.GetOptions) (
}
func (conn *Conn) UserGet(ctx context.Context, options arvados.GetOptions) (arvados.User, error) {
- resp, err := conn.chooseBackend(options.UUID).UserGet(ctx, options)
+ be := conn.chooseBackend(options.UUID)
+ resp, err := be.UserGet(ctx, options)
if err != nil {
return resp, err
}
@@ -795,7 +838,11 @@ func (conn *Conn) UserGet(ctx context.Context, options arvados.GetOptions) (arva
return arvados.User{}, httpErrorf(http.StatusBadGateway, "Had requested %v but response was for %v", options.UUID, resp.UUID)
}
if options.UUID[:5] != conn.cluster.ClusterID {
- err = conn.batchUpdateUsers(ctx, arvados.ListOptions{Select: options.Select}, []arvados.User{resp})
+ includeAdminAndInvited, err := conn.includeAdminAndInvitedInBatchUpdate(ctx, be, options.UUID)
+ if err != nil {
+ return arvados.User{}, err
+ }
+ err = conn.batchUpdateUsers(ctx, arvados.ListOptions{Select: options.Select}, []arvados.User{resp}, includeAdminAndInvited)
if err != nil {
return arvados.User{}, err
}
diff --git a/lib/controller/localdb/container_gateway.go b/lib/controller/localdb/container_gateway.go
index 376f55b7b3..0b6a630fae 100644
--- a/lib/controller/localdb/container_gateway.go
+++ b/lib/controller/localdb/container_gateway.go
@@ -349,7 +349,7 @@ func (conn *Conn) ContainerSSH(ctx context.Context, opts arvados.ContainerSSHOpt
return sshconn, err
}
ctxRoot := auth.NewContext(ctx, &auth.Credentials{Tokens: []string{conn.cluster.SystemRootToken}})
- if !*user.IsAdmin || !conn.cluster.Containers.ShellAccess.Admin {
+ if !user.IsAdmin || !conn.cluster.Containers.ShellAccess.Admin {
if !conn.cluster.Containers.ShellAccess.User {
return sshconn, httpserver.ErrorWithStatus(errors.New("shell access is disabled in config"), http.StatusServiceUnavailable)
}
diff --git a/sdk/go/arvados/user.go b/sdk/go/arvados/user.go
index 8dd71f2b72..2fb061e7fb 100644
--- a/sdk/go/arvados/user.go
+++ b/sdk/go/arvados/user.go
@@ -11,14 +11,14 @@ type User struct {
UUID string `json:"uuid"`
Etag string `json:"etag"`
IsActive bool `json:"is_active"`
- IsAdmin *bool `json:"is_admin,omitempty"`
+ IsAdmin bool `json:"is_admin"`
Username string `json:"username"`
Email string `json:"email"`
FullName string `json:"full_name"`
FirstName string `json:"first_name"`
LastName string `json:"last_name"`
IdentityURL string `json:"identity_url"`
- IsInvited *bool `json:"is_invited,omitempty"`
+ IsInvited bool `json:"is_invited"`
OwnerUUID string `json:"owner_uuid"`
CreatedAt time.Time `json:"created_at"`
ModifiedAt time.Time `json:"modified_at"`
diff --git a/services/keep-balance/balance.go b/services/keep-balance/balance.go
index 0b865d7475..e44dfeda87 100644
--- a/services/keep-balance/balance.go
+++ b/services/keep-balance/balance.go
@@ -267,7 +267,7 @@ func (bal *Balancer) CheckSanityEarly(c *arvados.Client) error {
if err != nil {
return fmt.Errorf("CurrentUser(): %v", err)
}
- if !u.IsActive || !*u.IsAdmin {
+ if !u.IsActive || !u.IsAdmin {
return fmt.Errorf("current user (%s) is not an active admin user", u.UUID)
}
for _, srv := range bal.KeepServices {
diff --git a/services/keep-web/handler.go b/services/keep-web/handler.go
index b5e0e7e896..123c4fe34d 100644
--- a/services/keep-web/handler.go
+++ b/services/keep-web/handler.go
@@ -851,7 +851,7 @@ func (h *handler) seeOtherWithCookie(w http.ResponseWriter, r *http.Request, loc
func (h *handler) userPermittedToUploadOrDownload(method string, tokenUser *arvados.User) bool {
var permitDownload bool
var permitUpload bool
- if tokenUser != nil && tokenUser.IsAdmin != nil && *tokenUser.IsAdmin {
+ if tokenUser != nil && tokenUser.IsAdmin {
permitUpload = h.Cluster.Collections.WebDAVPermission.Admin.Upload
permitDownload = h.Cluster.Collections.WebDAVPermission.Admin.Download
} else {
diff --git a/services/keepproxy/keepproxy.go b/services/keepproxy/keepproxy.go
index 964eaa7bdf..2090c50686 100644
--- a/services/keepproxy/keepproxy.go
+++ b/services/keepproxy/keepproxy.go
@@ -151,7 +151,7 @@ func (h *proxyHandler) checkAuthorizationHeader(req *http.Request) (pass bool, t
return false, "", nil
}
- if userCurrentError == nil && *user.IsAdmin {
+ 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 {
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list