mirror of https://github.com/minio/minio.git
				
				
				
			fix: load LDAP users appropriately (#9360)
This PR also fixes issues when deletePolicy, deleteUser is idempotent so can lead to issues when client can prematurely timeout, so a retry call error response should be ignored when call returns http.StatusNotFound Fixes #9347
This commit is contained in:
		
							parent
							
								
									a51280fd20
								
							
						
					
					
						commit
						c82fa2c829
					
				|  | @ -50,7 +50,11 @@ func readConfig(ctx context.Context, objAPI ObjectLayer, configFile string) ([]b | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| func deleteConfig(ctx context.Context, objAPI ObjectLayer, configFile string) error { | func deleteConfig(ctx context.Context, objAPI ObjectLayer, configFile string) error { | ||||||
| 	return objAPI.DeleteObject(ctx, minioMetaBucket, configFile) | 	err := objAPI.DeleteObject(ctx, minioMetaBucket, configFile) | ||||||
|  | 	if err != nil && isErrObjectNotFound(err) { | ||||||
|  | 		return errConfigNotFound | ||||||
|  | 	} | ||||||
|  | 	return err | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| func saveConfig(ctx context.Context, objAPI ObjectLayer, configFile string, data []byte) error { | func saveConfig(ctx context.Context, objAPI ObjectLayer, configFile string, data []byte) error { | ||||||
|  |  | ||||||
|  | @ -50,7 +50,6 @@ func handleEncryptedConfigBackend(objAPI ObjectLayer, server bool) error { | ||||||
| 
 | 
 | ||||||
| 	rquorum := InsufficientReadQuorum{} | 	rquorum := InsufficientReadQuorum{} | ||||||
| 	wquorum := InsufficientWriteQuorum{} | 	wquorum := InsufficientWriteQuorum{} | ||||||
| 	bucketNotFound := BucketNotFound{} |  | ||||||
| 
 | 
 | ||||||
| 	for !stop { | 	for !stop { | ||||||
| 		select { | 		select { | ||||||
|  | @ -58,7 +57,7 @@ func handleEncryptedConfigBackend(objAPI ObjectLayer, server bool) error { | ||||||
| 			if encrypted, err = checkBackendEncrypted(objAPI); err != nil { | 			if encrypted, err = checkBackendEncrypted(objAPI); err != nil { | ||||||
| 				if errors.Is(err, errDiskNotFound) || | 				if errors.Is(err, errDiskNotFound) || | ||||||
| 					errors.As(err, &rquorum) || | 					errors.As(err, &rquorum) || | ||||||
| 					errors.As(err, &bucketNotFound) { | 					isErrBucketNotFound(err) { | ||||||
| 					logger.Info("Waiting for config backend to be encrypted..") | 					logger.Info("Waiting for config backend to be encrypted..") | ||||||
| 					continue | 					continue | ||||||
| 				} | 				} | ||||||
|  | @ -108,7 +107,7 @@ func handleEncryptedConfigBackend(objAPI ObjectLayer, server bool) error { | ||||||
| 				if errors.Is(err, errDiskNotFound) || | 				if errors.Is(err, errDiskNotFound) || | ||||||
| 					errors.As(err, &rquorum) || | 					errors.As(err, &rquorum) || | ||||||
| 					errors.As(err, &wquorum) || | 					errors.As(err, &wquorum) || | ||||||
| 					errors.As(err, &bucketNotFound) { | 					isErrBucketNotFound(err) { | ||||||
| 					logger.Info("Waiting for config backend to be encrypted..") | 					logger.Info("Waiting for config backend to be encrypted..") | ||||||
| 					continue | 					continue | ||||||
| 				} | 				} | ||||||
|  |  | ||||||
|  | @ -233,11 +233,7 @@ func (iamOS *IAMObjectStore) loadIAMConfig(item interface{}, path string) error | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| func (iamOS *IAMObjectStore) deleteIAMConfig(path string) error { | func (iamOS *IAMObjectStore) deleteIAMConfig(path string) error { | ||||||
| 	err := deleteConfig(iamOS.ctx, iamOS.objAPI, path) | 	return deleteConfig(iamOS.ctx, iamOS.objAPI, path) | ||||||
| 	if _, ok := err.(ObjectNotFound); ok { |  | ||||||
| 		return errConfigNotFound |  | ||||||
| 	} |  | ||||||
| 	return err |  | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| func (iamOS *IAMObjectStore) loadPolicyDoc(policy string, m map[string]iampolicy.Policy) error { | func (iamOS *IAMObjectStore) loadPolicyDoc(policy string, m map[string]iampolicy.Policy) error { | ||||||
|  | @ -428,10 +424,12 @@ func (iamOS *IAMObjectStore) loadAll(ctx context.Context, sys *IAMSys) error { | ||||||
| 	if err := iamOS.loadPolicyDocs(ctx, iamPolicyDocsMap); err != nil { | 	if err := iamOS.loadPolicyDocs(ctx, iamPolicyDocsMap); err != nil { | ||||||
| 		return err | 		return err | ||||||
| 	} | 	} | ||||||
|  | 
 | ||||||
| 	// load STS temp users
 | 	// load STS temp users
 | ||||||
| 	if err := iamOS.loadUsers(ctx, stsUser, iamUsersMap); err != nil { | 	if err := iamOS.loadUsers(ctx, stsUser, iamUsersMap); err != nil { | ||||||
| 		return err | 		return err | ||||||
| 	} | 	} | ||||||
|  | 
 | ||||||
| 	if isMinIOUsersSys { | 	if isMinIOUsersSys { | ||||||
| 		if err := iamOS.loadUsers(ctx, regularUser, iamUsersMap); err != nil { | 		if err := iamOS.loadUsers(ctx, regularUser, iamUsersMap); err != nil { | ||||||
| 			return err | 			return err | ||||||
|  | @ -442,15 +440,18 @@ func (iamOS *IAMObjectStore) loadAll(ctx context.Context, sys *IAMSys) error { | ||||||
| 		if err := iamOS.loadGroups(ctx, iamGroupsMap); err != nil { | 		if err := iamOS.loadGroups(ctx, iamGroupsMap); err != nil { | ||||||
| 			return err | 			return err | ||||||
| 		} | 		} | ||||||
| 
 |  | ||||||
| 		if err := iamOS.loadMappedPolicies(ctx, regularUser, false, iamUserPolicyMap); err != nil { |  | ||||||
| 			return err |  | ||||||
| 		} |  | ||||||
| 	} | 	} | ||||||
|  | 
 | ||||||
|  | 	// load polices mapped to users
 | ||||||
|  | 	if err := iamOS.loadMappedPolicies(ctx, regularUser, false, iamUserPolicyMap); err != nil { | ||||||
|  | 		return err | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
| 	// load STS policy mappings
 | 	// load STS policy mappings
 | ||||||
| 	if err := iamOS.loadMappedPolicies(ctx, stsUser, false, iamUserPolicyMap); err != nil { | 	if err := iamOS.loadMappedPolicies(ctx, stsUser, false, iamUserPolicyMap); err != nil { | ||||||
| 		return err | 		return err | ||||||
| 	} | 	} | ||||||
|  | 
 | ||||||
| 	// load policies mapped to groups
 | 	// load policies mapped to groups
 | ||||||
| 	if err := iamOS.loadMappedPolicies(ctx, regularUser, true, iamGroupPolicyMap); err != nil { | 	if err := iamOS.loadMappedPolicies(ctx, regularUser, true, iamGroupPolicyMap); err != nil { | ||||||
| 		return err | 		return err | ||||||
|  |  | ||||||
							
								
								
									
										19
									
								
								cmd/iam.go
								
								
								
								
							
							
						
						
									
										19
									
								
								cmd/iam.go
								
								
								
								
							|  | @ -404,8 +404,7 @@ func (sys *IAMSys) DeletePolicy(policyName string) error { | ||||||
| 	defer sys.store.unlock() | 	defer sys.store.unlock() | ||||||
| 
 | 
 | ||||||
| 	err := sys.store.deletePolicyDoc(policyName) | 	err := sys.store.deletePolicyDoc(policyName) | ||||||
| 	switch err.(type) { | 	if err == errNoSuchPolicy { | ||||||
| 	case ObjectNotFound: |  | ||||||
| 		// Ignore error if policy is already deleted.
 | 		// Ignore error if policy is already deleted.
 | ||||||
| 		err = nil | 		err = nil | ||||||
| 	} | 	} | ||||||
|  | @ -417,7 +416,6 @@ func (sys *IAMSys) DeletePolicy(policyName string) error { | ||||||
| 	var usersType []IAMUserType | 	var usersType []IAMUserType | ||||||
| 	for u, mp := range sys.iamUserPolicyMap { | 	for u, mp := range sys.iamUserPolicyMap { | ||||||
| 		if mp.Policy == policyName { | 		if mp.Policy == policyName { | ||||||
| 			usersToDel = append(usersToDel, u) |  | ||||||
| 			cr, ok := sys.iamUsersMap[u] | 			cr, ok := sys.iamUsersMap[u] | ||||||
| 			if !ok { | 			if !ok { | ||||||
| 				// This case cannot happen
 | 				// This case cannot happen
 | ||||||
|  | @ -429,6 +427,7 @@ func (sys *IAMSys) DeletePolicy(policyName string) error { | ||||||
| 			} else { | 			} else { | ||||||
| 				usersType = append(usersType, regularUser) | 				usersType = append(usersType, regularUser) | ||||||
| 			} | 			} | ||||||
|  | 			usersToDel = append(usersToDel, u) | ||||||
| 		} | 		} | ||||||
| 	} | 	} | ||||||
| 	for i, u := range usersToDel { | 	for i, u := range usersToDel { | ||||||
|  | @ -538,8 +537,7 @@ func (sys *IAMSys) DeleteUser(accessKey string) error { | ||||||
| 	// It is ok to ignore deletion error on the mapped policy
 | 	// It is ok to ignore deletion error on the mapped policy
 | ||||||
| 	sys.store.deleteMappedPolicy(accessKey, regularUser, false) | 	sys.store.deleteMappedPolicy(accessKey, regularUser, false) | ||||||
| 	err := sys.store.deleteUserIdentity(accessKey, regularUser) | 	err := sys.store.deleteUserIdentity(accessKey, regularUser) | ||||||
| 	switch err.(type) { | 	if err == errNoSuchUser { | ||||||
| 	case ObjectNotFound: |  | ||||||
| 		// ignore if user is already deleted.
 | 		// ignore if user is already deleted.
 | ||||||
| 		err = nil | 		err = nil | ||||||
| 	} | 	} | ||||||
|  | @ -1020,14 +1018,11 @@ func (sys *IAMSys) RemoveUsersFromGroup(group string, members []string) error { | ||||||
| 		// len(gi.Members) == 0 here.
 | 		// len(gi.Members) == 0 here.
 | ||||||
| 
 | 
 | ||||||
| 		// Remove the group from storage. First delete the
 | 		// Remove the group from storage. First delete the
 | ||||||
| 		// mapped policy.
 | 		// mapped policy. No-mapped-policy case is ignored.
 | ||||||
| 		err := sys.store.deleteMappedPolicy(group, regularUser, true) | 		if err := sys.store.deleteMappedPolicy(group, regularUser, true); err != nil && err != errNoSuchPolicy { | ||||||
| 		// No-mapped-policy case is ignored.
 |  | ||||||
| 		if err != nil && err != errNoSuchPolicy { |  | ||||||
| 			return err | 			return err | ||||||
| 		} | 		} | ||||||
| 		err = sys.store.deleteGroupInfo(group) | 		if err := sys.store.deleteGroupInfo(group); err != nil && err != errNoSuchGroup { | ||||||
| 		if err != nil { |  | ||||||
| 			return err | 			return err | ||||||
| 		} | 		} | ||||||
| 
 | 
 | ||||||
|  | @ -1195,7 +1190,7 @@ func (sys *IAMSys) policyDBSet(name, policy string, userType IAMUserType, isGrou | ||||||
| 
 | 
 | ||||||
| 	// Handle policy mapping removal
 | 	// Handle policy mapping removal
 | ||||||
| 	if policy == "" { | 	if policy == "" { | ||||||
| 		if err := sys.store.deleteMappedPolicy(name, userType, isGroup); err != nil { | 		if err := sys.store.deleteMappedPolicy(name, userType, isGroup); err != nil && err != errNoSuchPolicy { | ||||||
| 			return err | 			return err | ||||||
| 		} | 		} | ||||||
| 		if !isGroup { | 		if !isGroup { | ||||||
|  |  | ||||||
|  | @ -168,6 +168,7 @@ func isS3CodeRetryable(s3Code string) (ok bool) { | ||||||
| 
 | 
 | ||||||
| // List of HTTP status codes which are retryable.
 | // List of HTTP status codes which are retryable.
 | ||||||
| var retryableHTTPStatusCodes = map[int]struct{}{ | var retryableHTTPStatusCodes = map[int]struct{}{ | ||||||
|  | 	http.StatusRequestTimeout:      {}, | ||||||
| 	http.StatusTooManyRequests:     {}, | 	http.StatusTooManyRequests:     {}, | ||||||
| 	http.StatusInternalServerError: {}, | 	http.StatusInternalServerError: {}, | ||||||
| 	http.StatusBadGateway:          {}, | 	http.StatusBadGateway:          {}, | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue