Bug report: Retrieving members for M365 group via entra m365group user list leads to unexpected result

[tmaestrini]

Priority

(Medium) I'm annoyed but I'll live

Description

As discussed with @milanholmans in Pull Request #6250, I've encountered an unexpected behavior while trying to retrieve the members from an m365 group.
While this seems to be normal behavior at first sight, digging into the code returned a filter statement that let me open this issue.

Steps to reproduce

Consider the following scenario: in my Entra ID, I have an M365 Group with id db8ac02c-fc83-4fca-b090-42f3cbe8b492. While the group consists of 20 members in total, there is only 1 owner and two external identities (aka "guests").

$allGroupMembers = m365 entra m365group user list --groupId "db8ac02c-fc83-4fca-b090-42f3cbe8b492" | ConvertFrom-Json
$allGroupMembers.Length  # returned 20 members

Members with the role member can be retrieved using following two approaches:

$membersRole = m365 entra m365group user list --groupId "db8ac02c-fc83-4fca-b090-42f3cbe8b492" --role Member | ConvertFrom-Json
$membersRole2 = $allGroupMembers | Where-Object { $_.roles -contains "Member" }

Expected results

Since we know that group owners also count as members, both approaches should return the same number of group members (which includes both owners and members as well as guests): 20.

Actual results

Unfortunately, the two approaches lead to different results:

$membersRole.Length # returns 19 members (when calling m365 entra m365group user list ...)
$membersRole2.Length # returns 20 members (when filtering the $allGroupMembers list) 

While $membersRole only contains members that match the role members, $membersRole2 also contains guests.

Diagnostics

The code analysis revealed that the command m365 entra m365group user list --role Member (see link to the git repo) contains a filter statement at the end of the corresponding method, which filters the result set for members along their userType (that was set via the role cli parameter):

public async commandAction(logger: Logger, args: CommandArgs): Promise<void> {
  // further code omitted
  if (!args.options.role || args.options.role === 'Member' || args.options.role === 'Guest') {
    const members = await this.getUsers(args.options, 'Members', groupId, logger);

    members.forEach((member: ExtendedUser) => {
      const user = users.find((u: ExtendedUser) => u.id === member.id);

      if (user !== undefined) {
        user.roles.push('Member');
      }
      else {
        users.push({ ...member, roles: ['Member'] });
      }
    });
  }

  // 👇 filtering the 'userType' property (see line 145 in the current code base) leads to the unexpected behavior
  if (args.options.role) {
    users = users.filter(i => i.userType === args.options.role);
  }
  // further code omitted
}

This leads to the unexpected result, which differs from this statement: $allGroupMembers | Where-Object { $_.roles -contains "Member" }, because filtering the result along the role option only returns a subset of all the members.

Conclusion

To me, filtering out the external identities from the result set when searching for group members totally makes sense on one hand (and is also a "convenient" way to having a "proper" list of members not containing any guest).
But on the other hand, as Microsoft includes the external users when displaying them in the group overview in Entra ID, the external users also should be included in the return value of the m365 cli command.

That said, imo either the documentation could be updated, or the filter clause should be removed.

What do you think? Am I missing something important?

CLI for Microsoft 365 version

v9.0.0

nodejs version

v18.19.1

Operating system (environment)

macOS

Shell

PowerShell

cli doctor

{
  "os": {
    "platform": "darwin",
    "version": "Darwin Kernel Version 23.5.0: Wed May  1 20:16:51 PDT 2024; root:xnu-10063.121.3~5/RELEASE_ARM64_T8103",
    "release": "23.5.0"
  },
  "cliVersion": "9.0.0",
  "nodeVersion": "v18.19.1",
  "cliAadAppId": "c2daee46-7596-4e73-adf3-4aa6d8d525ac",
  "cliAadAppTenant": "single",
  "authMode": "browser",
  "cliEnvironment": "",
  "cliConfig": {
    "authType": "browser",
    "autoOpenLinksInBrowser": false,
    "copyDeviceCodeToClipboard": false,
    "output": "json",
    "printErrorsAsPlainText": false,
    "prompt": false,
    "showHelpOnFailure": false,
    "showSpinner": false,
    "errorOutput": "stdout",
    "helpMode": "options",
    "clientId": "c2daee46-7596-4e73-adf3-4aa6d8d525ac",
    "tenantId": "15016ba4-ae9d-4b8c-90dd-54a31ca950b7"
  },
  "roles": [],
  "scopes": {
    "https://graph.microsoft.com": [
      "AllSites.FullControl",
      "AppCatalog.ReadWrite.All",
      "AuditLog.Read.All",
      "Bookings.Read.All",
      "Calendars.Read",
      "ChannelMember.ReadWrite.All",
      "ChannelMessage.Read.All",
      "ChannelMessage.ReadWrite",
      "ChannelMessage.Send",
      "ChannelSettings.ReadWrite.All",
      "Chat.ReadWrite",
      "Directory.AccessAsUser.All",
      "Directory.ReadWrite.All",
      "ExternalConnection.ReadWrite.All",
      "ExternalItem.ReadWrite.All",
      "Group.ReadWrite.All",
      "IdentityProvider.ReadWrite.All",
      "InformationProtectionPolicy.Read",
      "Mail.Read.Shared",
      "Mail.ReadWrite",
      "Mail.Send",
      "Notes.ReadWrite.All",
      "OnlineMeetingArtifact.Read.All",
      "OnlineMeetings.ReadWrite",
      "OnlineMeetingTranscript.Read.All",
      "PeopleSettings.ReadWrite.All",
      "Place.Read.All",
      "Policy.Read.All",
      "RecordsManagement.ReadWrite.All",
      "Reports.Read.All",
      "RoleAssignmentSchedule.ReadWrite.Directory",
      "RoleEligibilitySchedule.Read.Directory",
      "SecurityEvents.Read.All",
      "ServiceHealth.Read.All",
      "ServiceMessage.Read.All",
      "ServiceMessageViewpoint.Write",
      "Sites.Read.All",
      "Tasks.ReadWrite",
      "Team.Create",
      "TeamMember.ReadWrite.All",
      "TeamsAppInstallation.ReadWriteForUser",
      "TeamSettings.ReadWrite.All",
      "TeamsTab.ReadWrite.All",
      "TermStore.ReadWrite.All",
      "User.Invite.All",
      "User.ReadWrite.All",
      "profile",
      "openid",
      "email"
    ]
  }
}

Additional Info

No response

[Jwaegebaert]

Hey @tmaestrini, thanks for reporting this. What you're describing seems to be by design, so it's not really a bug. If you check the docs for entra m365group user list, you'll see three role types: owner, member, and guest. The outputs you're seeing make sense since fetching the user list with the member role excludes guests. This will change in v10 as the guest role is deprecated. So, the suggestion you made will be addressed in the new version, but for now, it's working as intended. The deprecation removal is tracked here: #5558.

[MartinM85]

@Jwaegebaert @waldekmastykarz

Let's say I have a group with 2 members in my tenant. One member is from my tenant, the second member is external (guest) user.

When running m365 entra m365group user list --role Member I would expect that both users will be returned.

await this.getUsers(args.options, 'Members', groupId, logger);

returns correctly both members, but then filtering by userType is applied and guest user is excluded from the result.

The issue is this filtering

if (args.options.role) {
  users = users.filter(i => i.userType === args.options.role);
}

userType is a property on user resource and can have either member or guest value (never owner).

I'm also worried about this line

https://github.com/pnp/cli-microsoft365/blob/main/src/m365/entra/commands/m365group/m365group-user-list.ts#L127

It overrides userType property coming from the Graph API and set it to Owner. What if a user wants to further process the results? The user expects that userType is either member or guest. I think it's not a good practice to override properties coming from Graph/SharePoint API.

[Jwaegebaert]

When running m365 entra m365group user list --role Member I would expect that both users will be returned.

This assumption isn't quite right because you can specify the guest role, meaning members wouldn't include guests. This works similarly to how UserType is defined in the user object of Graph. For clarity, we might have considered different naming for the role option, but as mentioned earlier, this logic will be updated in the upcoming major release.

It overrides userType property coming from the Graph API and set it to Owner. What if a user wants to further process the results? The user expects that userType is either member or guest. I think it's not a good practice to override properties coming from Graph/SharePoint API.

Good catch, we definitely shouldn't do that. If I’m not mistaken, this was updated in v10. Ref: eeee9e9#diff-b78e000d107217960c96ea8b2b26061328343b03bdf160d123cdc002e34053a6R123

[milanholemans]

It overrides userType property coming from the Graph API and set it to Owner. What if a user wants to further process the results? The user expects that userType is either member or guest. I think it's not a good practice to override properties coming from Graph/SharePoint API.

Like @Jwaegebaert mentioned the guest option is something we've fixed in v10 (#5558). We shouldn't override the userType property at all. When listing members of a group, the role option reflects the role of the user in that group being owner or member. The userType in the response is a property from the Entra user object and which we should leave untouched.

[tmaestrini]

Thanks to all!
As I've checked #5558 (v10), the filter statement (which was the key of my report)

if (args.options.role) {
  users = users.filter(i => i.userType === args.options.role);
}

was changed in line 142. 👍