fix: return mail and userPrincipalName for GroupMembership user members

fn.go

@@ -576,7 +576,10 @@ func (g *GraphQuery) fetchGroupMembers(ctx context.Context, client *msgraphsdk.G
 	// See: https://developer.microsoft.com/en-us/graph/known-issues/?search=25984
 	requestConfig := &groups.GroupItemRequestBuilderGetRequestConfiguration{
 		QueryParameters: &groups.GroupItemRequestBuilderGetQueryParameters{
-			Expand: []string{"members"},
+			// Explicitly select the standard member fields via a nested $select so
+			// that user properties such as mail and userPrincipalName are returned
+			// for the expanded members (see issue #115).
+			Expand: []string{"members($select=id,displayName,mail,userPrincipalName,appId)"},
 		},
 	}
 
@@ -639,24 +642,47 @@ func (g *GraphQuery) extractStringProperty(additionalData map[string]interface{}
 	return "", false
 }
 
-// extractUserProperties extracts user-specific properties from additionalData
-func (g *GraphQuery) extractUserProperties(additionalData map[string]interface{}, memberMap map[string]interface{}) {
-	// Extract mail property
-	if mail, ok := g.extractStringProperty(additionalData, "mail"); ok {
-		memberMap["mail"] = mail
+// extractUserProperties extracts user-specific properties.
+// Members expanded via $expand are deserialized into typed objects, so the
+// values live on the typed getters rather than in additionalData. We prefer
+// the typed getters and fall back to additionalData for plain DirectoryObjects.
+func (g *GraphQuery) extractUserProperties(member models.DirectoryObjectable, additionalData map[string]interface{}, memberMap map[string]interface{}) {
+	if user, ok := member.(models.Userable); ok {
+		if mail := ptr.Deref(user.GetMail(), ""); mail != "" {
+			memberMap["mail"] = mail
+		}
+		if upn := ptr.Deref(user.GetUserPrincipalName(), ""); upn != "" {
+			memberMap["userPrincipalName"] = upn
+		}
 	}
 
-	// Extract userPrincipalName property
-	if upn, ok := g.extractStringProperty(additionalData, "userPrincipalName"); ok {
-		memberMap["userPrincipalName"] = upn
+	// Fall back to additionalData when the typed getters did not provide a value.
+	if _, ok := memberMap["mail"]; !ok {
+		if mail, found := g.extractStringProperty(additionalData, "mail"); found {
+			memberMap["mail"] = mail
+		}
+	}
+	if _, ok := memberMap["userPrincipalName"]; !ok {
+		if upn, found := g.extractStringProperty(additionalData, "userPrincipalName"); found {
+			memberMap["userPrincipalName"] = upn
+		}
 	}
 }
 
-// extractServicePrincipalProperties extracts service principal specific properties
-func (g *GraphQuery) extractServicePrincipalProperties(additionalData map[string]interface{}, memberMap map[string]interface{}) {
-	// Extract appId property
-	if appID, ok := g.extractStringProperty(additionalData, "appId"); ok {
-		memberMap["appId"] = appID
+// extractServicePrincipalProperties extracts service principal specific properties.
+// As with users, prefer the typed getter and fall back to additionalData.
+func (g *GraphQuery) extractServicePrincipalProperties(member models.DirectoryObjectable, additionalData map[string]interface{}, memberMap map[string]interface{}) {
+	if sp, ok := member.(models.ServicePrincipalable); ok {
+		if appID := ptr.Deref(sp.GetAppId(), ""); appID != "" {
+			memberMap["appId"] = appID
+		}
+	}
+
+	// Fall back to additionalData when the typed getter did not provide a value.
+	if _, ok := memberMap["appId"]; !ok {
+		if appID, found := g.extractStringProperty(additionalData, "appId"); found {
+			memberMap["appId"] = appID
+		}
 	}
 }
 
@@ -710,9 +736,9 @@ func (g *GraphQuery) processMember(member models.DirectoryObjectable) map[string
 	// Extract type-specific properties
 	switch memberType {
 	case userType:
-		g.extractUserProperties(additionalData, memberMap)
+		g.extractUserProperties(member, additionalData, memberMap)
 	case servicePrincipalType:
-		g.extractServicePrincipalProperties(additionalData, memberMap)
+		g.extractServicePrincipalProperties(member, additionalData, memberMap)
 	}
 
 	return memberMap

fn_test.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/microsoftgraph/msgraph-sdk-go/models"
 	"github.com/upbound/function-msgraph/input/v1beta1"
 	"google.golang.org/protobuf/testing/protocmp"
 	"google.golang.org/protobuf/types/known/durationpb"
@@ -3815,3 +3816,113 @@ func TestIdentityType(t *testing.T) {
 		})
 	}
 }
+
+// newTestUser builds a typed user directory object, mirroring how the Graph SDK
+// deserializes expanded group members (the values land on the typed struct, not
+// in additionalData).
+func newTestUser() models.DirectoryObjectable {
+	user := models.NewUser()
+	user.SetId(ptr.To("user-id-1"))
+	user.SetDisplayName(ptr.To("Test User 1"))
+	user.SetMail(ptr.To("user1@example.com"))
+	user.SetUserPrincipalName(ptr.To("user1@example.com"))
+	return user
+}
+
+// newTestUserWithoutMail builds a typed user directory object that has no mail
+// attribute set (a common Entra ID state where the account has a UPN but the
+// mail attribute is null).
+func newTestUserWithoutMail() models.DirectoryObjectable {
+	user := models.NewUser()
+	user.SetId(ptr.To("user-id-3"))
+	user.SetDisplayName(ptr.To("No Mail User"))
+	user.SetUserPrincipalName(ptr.To("nomail@example.com"))
+	// mail intentionally left unset.
+	return user
+}
+
+// newTestServicePrincipal builds a typed service principal directory object.
+func newTestServicePrincipal() models.DirectoryObjectable {
+	sp := models.NewServicePrincipal()
+	sp.SetId(ptr.To("sp-id-1"))
+	sp.SetDisplayName(ptr.To("Test Service Principal"))
+	sp.SetAppId(ptr.To("sp-app-id-1"))
+	return sp
+}
+
+// newTestDirectoryObject builds a plain directory object whose properties are
+// only available via additionalData, exercising the fallback extraction path.
+func newTestDirectoryObject() models.DirectoryObjectable {
+	do := models.NewDirectoryObject()
+	do.SetId(ptr.To("user-id-2"))
+	do.SetAdditionalData(map[string]interface{}{
+		"displayName":       "Fallback User",
+		"mail":              "user2@example.com",
+		"userPrincipalName": "user2@example.com",
+	})
+	return do
+}
+
+// TestProcessMember exercises the real member-extraction path (bypassed by the
+// mocked graphQuery in the RunFunction tests). It is the regression test for
+// issue #115: typed user members must expose mail and userPrincipalName.
+func TestProcessMember(t *testing.T) {
+	cases := map[string]struct {
+		reason string
+		member models.DirectoryObjectable
+		want   map[string]interface{}
+	}{
+		"TypedUserIncludesMailAndUPN": {
+			reason: "A typed user member should expose mail and userPrincipalName from the typed getters",
+			member: newTestUser(),
+			want: map[string]interface{}{
+				"id":                "user-id-1",
+				"displayName":       "Test User 1",
+				"type":              "user",
+				"mail":              "user1@example.com",
+				"userPrincipalName": "user1@example.com",
+			},
+		},
+		"TypedUserWithoutMailOmitsMail": {
+			reason: "A typed user without a mail attribute should omit mail but still expose userPrincipalName",
+			member: newTestUserWithoutMail(),
+			want: map[string]interface{}{
+				"id":                "user-id-3",
+				"displayName":       "No Mail User",
+				"type":              "user",
+				"userPrincipalName": "nomail@example.com",
+			},
+		},
+		"TypedServicePrincipalIncludesAppID": {
+			reason: "A typed service principal member should expose appId from the typed getter",
+			member: newTestServicePrincipal(),
+			want: map[string]interface{}{
+				"id":          "sp-id-1",
+				"displayName": "Test Service Principal",
+				"type":        "servicePrincipal",
+				"appId":       "sp-app-id-1",
+			},
+		},
+		"PlainDirectoryObjectUsesAdditionalDataFallback": {
+			reason: "A plain directory object should fall back to additionalData for user properties",
+			member: newTestDirectoryObject(),
+			want: map[string]interface{}{
+				"id":                "user-id-2",
+				"displayName":       "Fallback User",
+				"type":              "user",
+				"mail":              "user2@example.com",
+				"userPrincipalName": "user2@example.com",
+			},
+		},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			g := &GraphQuery{log: logging.NewNopLogger()}
+			got := g.processMember(tc.member)
+			if diff := cmp.Diff(tc.want, got); diff != "" {
+				t.Errorf("%s\nprocessMember(...): -want, +got:\n%s", tc.reason, diff)
+			}
+		})
+	}
+}