From 9d96b18df03f5978873705ac55959161f018fb48 Mon Sep 17 00:00:00 2001 From: Aditya Manthramurthy Date: Wed, 17 May 2023 17:05:36 -0700 Subject: [PATCH] Add "name" and "description" params to service acc (#17172) --- cmd/admin-handlers-users.go | 47 +++++++++++++++++++++++++++--------- cmd/iam-etcd-store.go | 4 +++ cmd/iam-object-store.go | 4 +++ cmd/iam-store.go | 8 ++++-- cmd/iam.go | 15 ++++++------ cmd/site-replication.go | 12 ++++++--- go.mod | 4 +-- go.sum | 8 +++--- internal/auth/credentials.go | 8 +++++- 9 files changed, 79 insertions(+), 31 deletions(-) diff --git a/cmd/admin-handlers-users.go b/cmd/admin-handlers-users.go index 9f085946b..14ff94e36 100644 --- a/cmd/admin-handlers-users.go +++ b/cmd/admin-handlers-users.go @@ -665,6 +665,13 @@ func (a adminAPIHandlers) AddServiceAccount(w http.ResponseWriter, r *http.Reque return } + if err := createReq.Validate(); err != nil { + // Since this validation would happen client side as well, we only send + // a generic error message here. + writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(ErrAdminResourceInvalidArgument), r.URL) + return + } + var ( targetUser string targetGroups []string @@ -677,12 +684,17 @@ func (a adminAPIHandlers) AddServiceAccount(w http.ResponseWriter, r *http.Reque targetUser = cred.AccessKey } + description := createReq.Description + if description == "" { + description = createReq.Comment + } opts := newServiceAccountOpts{ - accessKey: createReq.AccessKey, - secretKey: createReq.SecretKey, - comment: createReq.Comment, - expiration: createReq.Expiration, - claims: make(map[string]interface{}), + accessKey: createReq.AccessKey, + secretKey: createReq.SecretKey, + name: createReq.Name, + description: description, + expiration: createReq.Expiration, + claims: make(map[string]interface{}), } // Find the user for the request sender (as it may be sent via a service @@ -835,7 +847,8 @@ func (a adminAPIHandlers) AddServiceAccount(w http.ResponseWriter, r *http.Reque AccessKey: newCred.AccessKey, SecretKey: newCred.SecretKey, Groups: newCred.Groups, - Comment: newCred.Comment, + Name: newCred.Name, + Description: newCred.Description, Claims: opts.claims, SessionPolicy: createReq.Policy, Status: auth.AccountOn, @@ -910,6 +923,13 @@ func (a adminAPIHandlers) UpdateServiceAccount(w http.ResponseWriter, r *http.Re return } + if err := updateReq.Validate(); err != nil { + // Since this validation would happen client side as well, we only send + // a generic error message here. + writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(ErrAdminResourceInvalidArgument), r.URL) + return + } + var sp *iampolicy.Policy if len(updateReq.NewPolicy) > 0 { sp, err = iampolicy.ParseConfig(bytes.NewReader(updateReq.NewPolicy)) @@ -921,7 +941,8 @@ func (a adminAPIHandlers) UpdateServiceAccount(w http.ResponseWriter, r *http.Re opts := updateServiceAccountOpts{ secretKey: updateReq.NewSecretKey, status: updateReq.NewStatus, - comment: updateReq.NewComment, + name: updateReq.NewName, + description: updateReq.NewDescription, expiration: updateReq.NewExpiration, sessionPolicy: sp, } @@ -940,7 +961,8 @@ func (a adminAPIHandlers) UpdateServiceAccount(w http.ResponseWriter, r *http.Re AccessKey: accessKey, SecretKey: opts.secretKey, Status: opts.status, - Comment: opts.comment, + Name: opts.name, + Description: opts.description, SessionPolicy: updateReq.NewPolicy, Expiration: updateReq.NewExpiration, }, @@ -1028,7 +1050,8 @@ func (a adminAPIHandlers) InfoServiceAccount(w http.ResponseWriter, r *http.Requ infoResp := madmin.InfoServiceAccountResp{ ParentUser: svcAccount.ParentUser, - Comment: svcAccount.Comment, + Name: svcAccount.Name, + Description: svcAccount.Description, AccountStatus: svcAccount.Status, ImpliedPolicy: policy == nil, Policy: string(policyJSON), @@ -2495,7 +2518,8 @@ func (a adminAPIHandlers) ImportIAM(w http.ResponseWriter, r *http.Request) { opts := updateServiceAccountOpts{ secretKey: svcAcctReq.SecretKey, status: svcAcctReq.Status, - comment: svcAcctReq.Comment, + name: svcAcctReq.Name, + description: svcAcctReq.Description, expiration: svcAcctReq.Expiration, sessionPolicy: sp, } @@ -2511,7 +2535,8 @@ func (a adminAPIHandlers) ImportIAM(w http.ResponseWriter, r *http.Request) { secretKey: svcAcctReq.SecretKey, sessionPolicy: sp, claims: svcAcctReq.Claims, - comment: svcAcctReq.Comment, + name: svcAcctReq.Name, + description: svcAcctReq.Description, expiration: svcAcctReq.Expiration, allowSiteReplicatorAccount: false, } diff --git a/cmd/iam-etcd-store.go b/cmd/iam-etcd-store.go index 6f84e69b2..f9e37c2f2 100644 --- a/cmd/iam-etcd-store.go +++ b/cmd/iam-etcd-store.go @@ -238,6 +238,10 @@ func (ies *IAMEtcdStore) addUser(ctx context.Context, user string, userType IAMU } u.Credentials.Claims = jwtClaims.Map() } + if u.Credentials.Description == "" { + u.Credentials.Description = u.Credentials.Comment + } + m[user] = u return nil } diff --git a/cmd/iam-object-store.go b/cmd/iam-object-store.go index 8f569d398..8b54b0a54 100644 --- a/cmd/iam-object-store.go +++ b/cmd/iam-object-store.go @@ -224,6 +224,10 @@ func (iamOS *IAMObjectStore) loadUser(ctx context.Context, user string, userType u.Credentials.Claims = jwtClaims.Map() } + if u.Credentials.Description == "" { + u.Credentials.Description = u.Credentials.Comment + } + m[user] = u return nil } diff --git a/cmd/iam-store.go b/cmd/iam-store.go index 57184f9f3..a38c30e33 100644 --- a/cmd/iam-store.go +++ b/cmd/iam-store.go @@ -2184,8 +2184,12 @@ func (store *IAMStoreSys) UpdateServiceAccount(ctx context.Context, accessKey st cr.SecretKey = opts.secretKey } - if opts.comment != "" { - cr.Comment = opts.comment + if opts.name != "" { + cr.Name = opts.name + } + + if opts.description != "" { + cr.Description = opts.description } if opts.expiration != nil { diff --git a/cmd/iam.go b/cmd/iam.go index 05351accf..0576a64d1 100644 --- a/cmd/iam.go +++ b/cmd/iam.go @@ -916,7 +916,7 @@ type newServiceAccountOpts struct { sessionPolicy *iampolicy.Policy accessKey string secretKey string - comment string + name, description string expiration *time.Time allowSiteReplicatorAccount bool // allow creating internal service account for site-replication. @@ -991,7 +991,8 @@ func (sys *IAMSys) NewServiceAccount(ctx context.Context, parentUser string, gro cred.ParentUser = parentUser cred.Groups = groups cred.Status = string(auth.AccountOn) - cred.Comment = opts.comment + cred.Name = opts.name + cred.Description = opts.description if opts.expiration != nil { expirationInUTC := opts.expiration.UTC() @@ -1011,11 +1012,11 @@ func (sys *IAMSys) NewServiceAccount(ctx context.Context, parentUser string, gro } type updateServiceAccountOpts struct { - sessionPolicy *iampolicy.Policy - secretKey string - status string - comment string - expiration *time.Time + sessionPolicy *iampolicy.Policy + secretKey string + status string + name, description string + expiration *time.Time } // UpdateServiceAccount - edit a service account diff --git a/cmd/site-replication.go b/cmd/site-replication.go index f5e965bfb..5c3383059 100644 --- a/cmd/site-replication.go +++ b/cmd/site-replication.go @@ -1222,7 +1222,8 @@ func (c *SiteReplicationSys) PeerSvcAccChangeHandler(ctx context.Context, change secretKey: change.Create.SecretKey, sessionPolicy: sp, claims: change.Create.Claims, - comment: change.Create.Comment, + name: change.Create.Name, + description: change.Create.Description, expiration: change.Create.Expiration, } _, _, err = globalIAMSys.NewServiceAccount(ctx, change.Create.Parent, change.Create.Groups, opts) @@ -1248,7 +1249,8 @@ func (c *SiteReplicationSys) PeerSvcAccChangeHandler(ctx context.Context, change opts := updateServiceAccountOpts{ secretKey: change.Update.SecretKey, status: change.Update.Status, - comment: change.Update.Comment, + name: change.Update.Name, + description: change.Update.Description, sessionPolicy: sp, expiration: change.Update.Expiration, } @@ -1852,7 +1854,8 @@ func (c *SiteReplicationSys) syncToAllPeers(ctx context.Context) error { Claims: claims, SessionPolicy: json.RawMessage(policyJSON), Status: acc.Credentials.Status, - Comment: acc.Credentials.Comment, + Name: acc.Credentials.Name, + Description: acc.Credentials.Description, Expiration: &acc.Credentials.Expiration, }, }, @@ -4737,7 +4740,8 @@ func (c *SiteReplicationSys) healUsers(ctx context.Context, objAPI ObjectLayer, Claims: claims, SessionPolicy: json.RawMessage(policyJSON), Status: creds.Status, - Comment: creds.Comment, + Name: creds.Name, + Description: creds.Description, Expiration: &creds.Expiration, }, }, diff --git a/go.mod b/go.mod index 2b8f96f8e..53ad25ae8 100644 --- a/go.mod +++ b/go.mod @@ -48,7 +48,7 @@ require ( github.com/minio/dperf v0.4.2 github.com/minio/highwayhash v1.0.2 github.com/minio/kes-go v0.1.0 - github.com/minio/madmin-go/v2 v2.1.1 + github.com/minio/madmin-go/v2 v2.1.3 github.com/minio/minio-go/v7 v7.0.52 github.com/minio/mux v1.9.0 github.com/minio/pkg v1.7.1 @@ -177,7 +177,7 @@ require ( github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect github.com/minio/colorjson v1.0.4 // indirect github.com/minio/filepath v1.0.0 // indirect - github.com/minio/mc v0.0.0-20230509151326-6050568e66a6 // indirect + github.com/minio/mc v0.0.0-20230517184609-052cf01a9604 // indirect github.com/minio/md5-simd v1.1.2 // indirect github.com/minio/websocket v1.6.0 // indirect github.com/mitchellh/mapstructure v1.5.0 // indirect diff --git a/go.sum b/go.sum index c1a369445..6afb3f2ff 100644 --- a/go.sum +++ b/go.sum @@ -778,10 +778,10 @@ github.com/minio/highwayhash v1.0.2/go.mod h1:BQskDq+xkJ12lmlUUi7U0M5Swg3EWR+dLT github.com/minio/kes-go v0.1.0 h1:h201DyOYP5sTqajkxFGxmXz/kPbT8HQNX1uh3Yx2PFc= github.com/minio/kes-go v0.1.0/go.mod h1:VorHLaIYis9/MxAHAtXN4d8PUMNKhIxTIlvFt0hBOEo= github.com/minio/madmin-go v1.6.6/go.mod h1:ATvkBOLiP3av4D++2v1UEHC/QzsGtgXD5kYvvRYzdKs= -github.com/minio/madmin-go/v2 v2.1.1 h1:qUdJP31MD3ThPOmvfRby0J5ZJdAw5XK67LxTkUI9DWA= -github.com/minio/madmin-go/v2 v2.1.1/go.mod h1:8bL1RMNkblIENFSgGYjeHrzUx9PxROb7OqfNuMU9ivE= -github.com/minio/mc v0.0.0-20230509151326-6050568e66a6 h1:bRJ1PnUl58yZ4gUlHyCWexmv6jxBBfq1pmjrt1PERHw= -github.com/minio/mc v0.0.0-20230509151326-6050568e66a6/go.mod h1:qs/xdw+kX2neHHthlUZ5pHc0RUKwxlzcaZfAkEKpmSI= +github.com/minio/madmin-go/v2 v2.1.3 h1:9pkUgAujfm/SaFei4a1LwpS2et1/qGvRjFqFbRWa6xA= +github.com/minio/madmin-go/v2 v2.1.3/go.mod h1:8bL1RMNkblIENFSgGYjeHrzUx9PxROb7OqfNuMU9ivE= +github.com/minio/mc v0.0.0-20230517184609-052cf01a9604 h1:4CKGq+0QrDi1SdqTFZFXxAFReukoig+ElLRezpDd7+E= +github.com/minio/mc v0.0.0-20230517184609-052cf01a9604/go.mod h1:RE5bLggH7ehQUDoGYKW8B5h7kFrYRRNF/IAXOdF8Ch8= github.com/minio/md5-simd v1.1.2 h1:Gdi1DZK69+ZVMoNHRXJyNcxrMA4dSxoYHZSQbirFg34= github.com/minio/md5-simd v1.1.2/go.mod h1:MzdKDxYpY2BT9XQFocsiZf/NKVtR7nkE4RoEpN+20RM= github.com/minio/minio-go/v6 v6.0.46/go.mod h1:qD0lajrGW49lKZLtXKtCB4X/qkMf0a5tBvN2PaZg7Gg= diff --git a/internal/auth/credentials.go b/internal/auth/credentials.go index 9d4e4653a..90a2b8630 100644 --- a/internal/auth/credentials.go +++ b/internal/auth/credentials.go @@ -108,7 +108,13 @@ type Credentials struct { ParentUser string `xml:"-" json:"parentUser,omitempty"` Groups []string `xml:"-" json:"groups,omitempty"` Claims map[string]interface{} `xml:"-" json:"claims,omitempty"` - Comment string `xml:"-" json:"comment,omitempty"` + Name string `xml:"-" json:"name,omitempty"` + Description string `xml:"-" json:"description,omitempty"` + + // Deprecated: In favor of Description - when reading credentials from + // storage the value of this field is placed in the Description field above + // if the existing Description from storage is empty. + Comment string `xml:"-" json:"comment,omitempty"` } func (cred Credentials) String() string {