From 0ee722f8c3b3d8b566c6d4b648d71b65bbf3a54b Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Wed, 29 Nov 2023 16:07:35 -0800 Subject: [PATCH] cleanup handling of STS isAllowed and simplifies the PolicyDBGet() (#18554) --- cmd/admin-handlers-users.go | 17 ++++++++++++----- cmd/ftp-server-driver.go | 4 ++-- cmd/iam-store.go | 19 ++++++++++--------- cmd/iam.go | 10 +++++----- cmd/sftp-server.go | 2 +- cmd/sts-handlers.go | 4 ++-- 6 files changed, 32 insertions(+), 24 deletions(-) diff --git a/cmd/admin-handlers-users.go b/cmd/admin-handlers-users.go index 5304590d8..4fa38de49 100644 --- a/cmd/admin-handlers-users.go +++ b/cmd/admin-handlers-users.go @@ -545,14 +545,16 @@ func (a adminAPIHandlers) TemporaryAccountInfo(w http.ResponseWriter, r *http.Re return } - if !globalIAMSys.IsAllowed(policy.Args{ + args := policy.Args{ AccountName: cred.AccessKey, Groups: cred.Groups, Action: policy.ListTemporaryAccountsAdminAction, ConditionValues: getConditionValues(r, "", cred), IsOwner: owner, Claims: cred.Claims, - }) { + } + + if !globalIAMSys.IsAllowed(args) { writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(ErrAccessDenied), r.URL) return } @@ -568,11 +570,16 @@ func (a adminAPIHandlers) TemporaryAccountInfo(w http.ResponseWriter, r *http.Re if sessionPolicy != nil { stsAccountPolicy = *sessionPolicy } else { - policiesNames, err := globalIAMSys.PolicyDBGet(stsAccount.ParentUser, false) + policiesNames, err := globalIAMSys.PolicyDBGet(stsAccount.ParentUser, stsAccount.Groups...) if err != nil { writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL) return } + if len(policiesNames) == 0 { + policySet, _ := args.GetPolicies(iamPolicyClaimNameOpenID()) + policiesNames = policySet.ToSlice() + } + stsAccountPolicy = globalIAMSys.GetCombinedPolicy(policiesNames...) } @@ -1010,7 +1017,7 @@ func (a adminAPIHandlers) InfoServiceAccount(w http.ResponseWriter, r *http.Requ if !impliedPolicy { svcAccountPolicy = *sessionPolicy } else { - policiesNames, err := globalIAMSys.PolicyDBGet(svcAccount.ParentUser, false) + policiesNames, err := globalIAMSys.PolicyDBGet(svcAccount.ParentUser, svcAccount.Groups...) if err != nil { writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL) return @@ -1357,7 +1364,7 @@ func (a adminAPIHandlers) AccountInfoHandler(w http.ResponseWriter, r *http.Requ effectivePolicy = globalIAMSys.GetCombinedPolicy(policySetFromClaims.ToSlice()...) default: - policies, err := globalIAMSys.PolicyDBGet(accountName, false, cred.Groups...) + policies, err := globalIAMSys.PolicyDBGet(accountName, cred.Groups...) if err != nil { logger.LogIf(ctx, err) writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL) diff --git a/cmd/ftp-server-driver.go b/cmd/ftp-server-driver.go index 35ea43c50..f62775e19 100644 --- a/cmd/ftp-server-driver.go +++ b/cmd/ftp-server-driver.go @@ -252,7 +252,7 @@ func (driver *ftpDriver) CheckPasswd(c *ftp.Context, username, password string) if err != nil { return false, err } - ldapPolicies, _ := globalIAMSys.PolicyDBGet(ldapUserDN, false, groupDistNames...) + ldapPolicies, _ := globalIAMSys.PolicyDBGet(ldapUserDN, groupDistNames...) return len(ldapPolicies) > 0, nil } @@ -273,7 +273,7 @@ func (driver *ftpDriver) getMinIOClient(ctx *ftp.Context) (*minio.Client, error) if err != nil { return nil, err } - ldapPolicies, _ := globalIAMSys.PolicyDBGet(targetUser, false, targetGroups...) + ldapPolicies, _ := globalIAMSys.PolicyDBGet(targetUser, targetGroups...) if len(ldapPolicies) == 0 { return nil, errAuthentication } diff --git a/cmd/iam-store.go b/cmd/iam-store.go index 8c172a3aa..734b0780f 100644 --- a/cmd/iam-store.go +++ b/cmd/iam-store.go @@ -655,7 +655,7 @@ func (store *IAMStoreSys) GroupNotificationHandler(ctx context.Context, group st // PolicyDBGet - fetches policies associated with the given user or group, and // additional groups if provided. -func (store *IAMStoreSys) PolicyDBGet(name string, isGroup bool, groups ...string) ([]string, error) { +func (store *IAMStoreSys) PolicyDBGet(name string, groups ...string) ([]string, error) { if name == "" { return nil, errInvalidArgument } @@ -663,19 +663,17 @@ func (store *IAMStoreSys) PolicyDBGet(name string, isGroup bool, groups ...strin cache := store.rlock() defer store.runlock() - policies, _, err := cache.policyDBGet(store, name, isGroup) + policies, _, err := cache.policyDBGet(store, name, false) if err != nil { return nil, err } - if !isGroup { - for _, group := range groups { - ps, _, err := cache.policyDBGet(store, group, true) - if err != nil { - return nil, err - } - policies = append(policies, ps...) + for _, group := range groups { + ps, _, err := cache.policyDBGet(store, group, true) + if err != nil { + return nil, err } + policies = append(policies, ps...) } return policies, nil @@ -1219,6 +1217,9 @@ func (store *IAMStoreSys) GetPolicy(name string) (policy.Policy, error) { } toMerge = append(toMerge, v.Policy) } + if len(toMerge) == 0 { + return policy.Policy{}, errNoSuchPolicy + } return policy.MergePolicies(toMerge...), nil } diff --git a/cmd/iam.go b/cmd/iam.go index 2feadabe1..c1951733d 100644 --- a/cmd/iam.go +++ b/cmd/iam.go @@ -1720,12 +1720,12 @@ func (sys *IAMSys) PolicyDBUpdateLDAP(ctx context.Context, isAttach bool, // PolicyDBGet - gets policy set on a user or group. If a list of groups is // given, policies associated with them are included as well. -func (sys *IAMSys) PolicyDBGet(name string, isGroup bool, groups ...string) ([]string, error) { +func (sys *IAMSys) PolicyDBGet(name string, groups ...string) ([]string, error) { if !sys.Initialized() { return nil, errServerNotInitialized } - return sys.store.PolicyDBGet(name, isGroup, groups...) + return sys.store.PolicyDBGet(name, groups...) } const sessionPolicyNameExtracted = policy.SessionPolicyName + "-extracted" @@ -1774,7 +1774,7 @@ func (sys *IAMSys) IsAllowedServiceAccount(args policy.Args, parentUser string) default: // Check policy for parent user of service account. - svcPolicies, err = sys.PolicyDBGet(parentUser, false, args.Groups...) + svcPolicies, err = sys.PolicyDBGet(parentUser, args.Groups...) if err != nil { logger.LogIf(GlobalContext, err) return false @@ -1882,7 +1882,7 @@ func (sys *IAMSys) IsAllowedSTS(args policy.Args, parentUser string) bool { default: // Otherwise, inherit parent user's policy var err error - policies, err = sys.store.PolicyDBGet(parentUser, false, args.Groups...) + policies, err = sys.store.PolicyDBGet(parentUser, args.Groups...) if err != nil { logger.LogIf(GlobalContext, fmt.Errorf("error fetching policies on %s: %v", parentUser, err)) return false @@ -2019,7 +2019,7 @@ func (sys *IAMSys) IsAllowed(args policy.Args) bool { } // Continue with the assumption of a regular user - policies, err := sys.PolicyDBGet(args.AccountName, false, args.Groups...) + policies, err := sys.PolicyDBGet(args.AccountName, args.Groups...) if err != nil { return false } diff --git a/cmd/sftp-server.go b/cmd/sftp-server.go index 56bd78dd3..c5a521e24 100644 --- a/cmd/sftp-server.go +++ b/cmd/sftp-server.go @@ -114,7 +114,7 @@ func startSFTPServer(c *cli.Context) { if err != nil { return nil, err } - ldapPolicies, _ := globalIAMSys.PolicyDBGet(targetUser, false, targetGroups...) + ldapPolicies, _ := globalIAMSys.PolicyDBGet(targetUser, targetGroups...) if len(ldapPolicies) == 0 { return nil, errAuthentication } diff --git a/cmd/sts-handlers.go b/cmd/sts-handlers.go index beb05370e..b57340df6 100644 --- a/cmd/sts-handlers.go +++ b/cmd/sts-handlers.go @@ -267,7 +267,7 @@ func (sts *stsAPIHandlers) AssumeRole(w http.ResponseWriter, r *http.Request) { // Validate that user.AccessKey's policies can be retrieved - it may not // be in case the user is disabled. - if _, err = globalIAMSys.PolicyDBGet(user.AccessKey, false); err != nil { + if _, err = globalIAMSys.PolicyDBGet(user.AccessKey, user.Groups...); err != nil { writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue, err) return } @@ -630,7 +630,7 @@ func (sts *stsAPIHandlers) AssumeRoleWithLDAPIdentity(w http.ResponseWriter, r * } // Check if this user or their groups have a policy applied. - ldapPolicies, _ := globalIAMSys.PolicyDBGet(ldapUserDN, false, groupDistNames...) + ldapPolicies, _ := globalIAMSys.PolicyDBGet(ldapUserDN, groupDistNames...) if len(ldapPolicies) == 0 && newGlobalAuthZPluginFn() == nil { writeSTSErrorResponse(ctx, w, ErrSTSInvalidParameterValue, fmt.Errorf("expecting a policy to be set for user `%s` or one of their groups: `%s` - rejecting this request",