From 307ba613446309ca7b77bd0ce290e143c269b59c Mon Sep 17 00:00:00 2001
From: Tiago Silva <tiago.silva@goteleport.com>
Date: Wed, 30 Oct 2024 22:24:31 +0000
Subject: [PATCH] [entraid] ignore unsuported groups members and parse nested
 group memberships (#48182)

This PR fixes a typo where the error was incorrectly ignored and caused failures when group membership included other groups and any unsupported kinds.

This PR fixes that by properly returning `unsupportedGroupMember` while also supports parsing groups that are member of other groups.

Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>
---
 lib/msgraph/client_test.go | 62 ++++++++++++++++++++++++++++++++++++++
 lib/msgraph/models.go      |  8 ++++-
 2 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/lib/msgraph/client_test.go b/lib/msgraph/client_test.go
index 8df3eb7852d81..8e49692249c0a 100644
--- a/lib/msgraph/client_test.go
+++ b/lib/msgraph/client_test.go
@@ -412,3 +412,65 @@ func TestRetry(t *testing.T) {
 		require.Error(t, err)
 	})
 }
+
+const listGroupsMembersPayload = `[
+    {
+      "@odata.type": "#microsoft.graph.user",
+      "id": "9f615773-8219-4a5e-9eb1-8e701324c683",
+      "mail": "alice@example.com"
+    },
+	{
+      "@odata.type": "#microsoft.graph.device",
+      "id": "1566d9a7-c652-44e7-a75e-665b77431435",
+      "mail": "device@example.com"
+    },
+	{
+      "@odata.type": "#microsoft.graph.group",
+      "id": "7db727c5-924a-4f6d-b1f0-d44e6cafa87c",
+      "displayName": "Test Group 1"
+    }
+  ]`
+
+func TestIterateGroupMembers(t *testing.T) {
+	t.Parallel()
+
+	var membersJSON []json.RawMessage
+	require.NoError(t, json.Unmarshal([]byte(listGroupsMembersPayload), &membersJSON))
+	mux := http.NewServeMux()
+	groupID := "fd5be192-6e51-4f54-bbdf-30407435ceb7"
+	mux.Handle("GET /groups/"+groupID+"/members", paginatedHandler(t, membersJSON))
+
+	srv := httptest.NewServer(mux)
+	t.Cleanup(func() { srv.Close() })
+
+	uri, err := url.Parse(srv.URL)
+	require.NoError(t, err)
+	client := &Client{
+		httpClient:    &http.Client{},
+		tokenProvider: &fakeTokenProvider{},
+		retryConfig:   retryConfig,
+		baseURL:       uri,
+		pageSize:      2, // smaller page size so we actually fetch multiple pages with our small test payload
+	}
+
+	var members []GroupMember
+	err = client.IterateGroupMembers(context.Background(), groupID, func(u GroupMember) bool {
+		members = append(members, u)
+		return true
+	})
+
+	require.NoError(t, err)
+	require.Len(t, members, 2)
+	{
+		require.IsType(t, &User{}, members[0])
+		user := members[0].(*User)
+		require.Equal(t, "9f615773-8219-4a5e-9eb1-8e701324c683", *user.ID)
+		require.Equal(t, "alice@example.com", *user.Mail)
+	}
+	{
+		require.IsType(t, &Group{}, members[1])
+		group := members[1].(*Group)
+		require.Equal(t, "7db727c5-924a-4f6d-b1f0-d44e6cafa87c", *group.ID)
+		require.Equal(t, "Test Group 1", *group.DisplayName)
+	}
+}
diff --git a/lib/msgraph/models.go b/lib/msgraph/models.go
index 829d55a040464..1150c8943e7d1 100644
--- a/lib/msgraph/models.go
+++ b/lib/msgraph/models.go
@@ -116,8 +116,14 @@ func decodeGroupMember(msg json.RawMessage) (GroupMember, error) {
 		var u *User
 		err = json.Unmarshal(msg, &u)
 		member = u
+	case "#microsoft.graph.group":
+		var g *Group
+		err = json.Unmarshal(msg, &g)
+		member = g
 	default:
-		err = trace.BadParameter("unsupported group member type: %s", temp.Type)
+		// Return an error if we encounter a type we do not support.
+		// The caller ignores the error and continues processing the next entry.
+		err = &unsupportedGroupMember{Type: temp.Type}
 	}
 
 	return member, trace.Wrap(err)