From 84e122c5c3bc32767772569d483603e6220b9275 Mon Sep 17 00:00:00 2001
From: Taran Pelkey <tpelkey2021@gmail.com>
Date: Fri, 6 Sep 2024 20:28:47 -0400
Subject: [PATCH] Fix duplicate groups in ListGroups API (#20396)

---
 cmd/admin-handlers-users_test.go |  6 ++++--
 cmd/iam-store.go                 | 13 +++++++------
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/cmd/admin-handlers-users_test.go b/cmd/admin-handlers-users_test.go
index f4082ce80..a86411940 100644
--- a/cmd/admin-handlers-users_test.go
+++ b/cmd/admin-handlers-users_test.go
@@ -28,6 +28,7 @@ import (
 	"net/http"
 	"net/url"
 	"runtime"
+	"slices"
 	"strings"
 	"testing"
 	"time"
@@ -770,8 +771,9 @@ func (s *TestSuiteIAM) TestGroupAddRemove(c *check) {
 	if err != nil {
 		c.Fatalf("group list err: %v", err)
 	}
-	if !set.CreateStringSet(groups...).Contains(group) {
-		c.Fatalf("created group not present!")
+	expected := []string{group}
+	if !slices.Equal(groups, expected) {
+		c.Fatalf("expected group listing: %v, got: %v", expected, groups)
 	}
 	groupInfo, err := s.adm.GetGroupDescription(ctx, group)
 	if err != nil {
diff --git a/cmd/iam-store.go b/cmd/iam-store.go
index 9435db0da..df4ea08b7 100644
--- a/cmd/iam-store.go
+++ b/cmd/iam-store.go
@@ -980,6 +980,7 @@ func (store *IAMStoreSys) GetGroupDescription(group string) (gd madmin.GroupDesc
 
 // updateGroups updates the group from the persistent store, and also related policy mapping if any.
 func (store *IAMStoreSys) updateGroups(ctx context.Context, cache *iamCache) (res []string, err error) {
+	groupSet := set.NewStringSet()
 	if iamOS, ok := store.IAMStorageAPI.(*IAMObjectStore); ok {
 		listedConfigItems, err := iamOS.listAllIAMConfigItems(ctx)
 		if err != nil {
@@ -992,7 +993,7 @@ func (store *IAMStoreSys) updateGroups(ctx context.Context, cache *iamCache) (re
 				if err = iamOS.loadGroup(ctx, group, cache.iamGroupsMap); err != nil && !errors.Is(err, errNoSuchGroup) {
 					return nil, fmt.Errorf("unable to load the group: %w", err)
 				}
-				res = append(res, group)
+				groupSet.Add(group)
 			}
 		}
 
@@ -1002,23 +1003,23 @@ func (store *IAMStoreSys) updateGroups(ctx context.Context, cache *iamCache) (re
 			if err = iamOS.loadMappedPolicy(ctx, group, regUser, true, cache.iamGroupPolicyMap); err != nil && !errors.Is(err, errNoSuchPolicy) {
 				return nil, fmt.Errorf("unable to load the policy mapping for the group: %w", err)
 			}
-			res = append(res, group)
+			groupSet.Add(group)
 		}
 
-		return res, nil
+		return groupSet.ToSlice(), nil
 	}
 
 	// For etcd just return from cache.
 	for k := range cache.iamGroupsMap {
-		res = append(res, k)
+		groupSet.Add(k)
 	}
 
 	cache.iamGroupPolicyMap.Range(func(k string, v MappedPolicy) bool {
-		res = append(res, k)
+		groupSet.Add(k)
 		return true
 	})
 
-	return res, nil
+	return groupSet.ToSlice(), nil
 }
 
 // ListGroups - lists groups. Since this is not going to be a frequent