review comments

pnp · Oct 19, 2023 · 15c6b3f · 15c6b3f
1 parent 4fbb54d
commit 15c6b3f
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 53 deletions.
diff --git a/docs/docs/cmd/aad/group/group-user-list.mdx b/docs/docs/cmd/aad/group/group-user-list.mdx
@@ -33,6 +33,10 @@ m365 aad group user list [options]
 
 <Global />
 
+## Remarks
+
+When the `properties` option includes values with a `/`, for example: `memberof/id`, an additional `$expand` query parameter will be included on `memberof`.
+
 ## Examples
 
 List all group users from a group specified by ID.
@@ -47,10 +51,10 @@ List all owners from a group specified by display name.
 m365 aad group user list --groupDisplayName Developers --role Owner
 ```
 
-List all group users from a group specified by name. For each one return the display name and e-mail address
+List all group users from a group specified by name. For each one return the display name, e-mail address, and memberof.
 
 ```sh
-m365 aad group user list --groupDisplayName Developers --properties "displayName,mail"
+m365 aad group user list --groupDisplayName Developers --properties "displayName,mail,memberof/id"
 ```
 
 List all group members that are guest users.

diff --git a/src/m365/aad/commands/group/group-user-list.spec.ts b/src/m365/aad/commands/group/group-user-list.spec.ts
@@ -103,13 +103,13 @@ describe(commands.GROUP_USER_LIST, () => {
 
   it('correctly lists all users in a Azure AD group by id', async () => {
     sinon.stub(request, 'get').callsFake(async (opts) => {
-      if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Owners?$select=id,displayName,userPrincipalName,givenName,surname`) {
+      if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Owners/microsoft.graph.user?$select=id,displayName,userPrincipalName,givenName,surname`) {
         return {
           "value": [{ "id": "00000000-0000-0000-0000-000000000000", "displayName": "Anne Matthews", "userPrincipalName": "anne.matthews@contoso.onmicrosoft.com", "givenName": "Anne", "surname": "Matthews" }]
         };
       }
 
-      if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Members?$select=id,displayName,userPrincipalName,givenName,surname`) {
+      if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Members/microsoft.graph.user?$select=id,displayName,userPrincipalName,givenName,surname`) {
         return {
           "value": [
             { "id": "00000000-0000-0000-0000-000000000000", "displayName": "Anne Matthews", "userPrincipalName": "anne.matthews@contoso.onmicrosoft.com", "givenName": "Anne", "surname": "Matthews" },
@@ -147,13 +147,13 @@ describe(commands.GROUP_USER_LIST, () => {
     sinon.stub(aadGroup, 'getGroupIdByDisplayName').resolves(groupId);
 
     sinon.stub(request, 'get').callsFake(async (opts) => {
-      if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Owners?$select=id,displayName,userPrincipalName,givenName,surname`) {
+      if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Owners/microsoft.graph.user?$select=id,displayName,userPrincipalName,givenName,surname`) {
         return {
           "value": [{ "id": "00000000-0000-0000-0000-000000000000", "displayName": "Anne Matthews", "userPrincipalName": "anne.matthews@contoso.onmicrosoft.com", "givenName": "Anne", "surname": "Matthews" }]
         };
       }
 
-      if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Members?$select=id,displayName,userPrincipalName,givenName,surname`) {
+      if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Members/microsoft.graph.user?$select=id,displayName,userPrincipalName,givenName,surname`) {
         return {
           "value": [
             { "id": "00000000-0000-0000-0000-000000000000", "displayName": "Anne Matthews", "userPrincipalName": "anne.matthews@contoso.onmicrosoft.com", "givenName": "Anne", "surname": "Matthews" },
@@ -189,7 +189,7 @@ describe(commands.GROUP_USER_LIST, () => {
 
   it('correctly lists all owners in a Azure AD group', async () => {
     sinon.stub(request, 'get').callsFake(async (opts) => {
-      if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Owners?$select=id,displayName,userPrincipalName,givenName,surname`) {
+      if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Owners/microsoft.graph.user?$select=id,displayName,userPrincipalName,givenName,surname`) {
         return {
           "value": [{ "id": "00000000-0000-0000-0000-000000000000", "displayName": "Anne Matthews", "userPrincipalName": "anne.matthews@contoso.onmicrosoft.com", "givenName": "Anne", "surname": "Matthews" }]
         };
@@ -212,7 +212,7 @@ describe(commands.GROUP_USER_LIST, () => {
 
   it('correctly lists all members in a Azure AD group', async () => {
     sinon.stub(request, 'get').callsFake(async (opts) => {
-      if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Members?$select=id,displayName,userPrincipalName,givenName,surname`) {
+      if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Members/microsoft.graph.user?$select=id,displayName,userPrincipalName,givenName,surname`) {
         return {
           "value": [
             { "id": "00000000-0000-0000-0000-000000000000", "displayName": "Anne Matthews", "userPrincipalName": "anne.matthews@contoso.onmicrosoft.com", "givenName": "Anne", "surname": "Matthews" },
@@ -248,42 +248,42 @@ describe(commands.GROUP_USER_LIST, () => {
 
   it('correctly lists properties for all users in a Azure AD group', async () => {
     sinon.stub(request, 'get').callsFake(async (opts) => {
-      if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Owners?$select=displayName,mail,id`) {
+      if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Owners/microsoft.graph.user?$select=displayName,mail,id&$expand=memberof($select=id),memberof($select=displayName)`) {
         return {
           "value": [
-            { "id": "00000000-0000-0000-0000-000000000000", "displayName": "Karl Matteson", "mail": "karl.matteson@contoso.onmicrosoft.com" }
+            { "id": "00000000-0000-0000-0000-000000000000", "displayName": "Karl Matteson", "mail": "karl.matteson@contoso.onmicrosoft.com", "memberOf": [{ "displayName": "Life and Music", "id": "d6c88284-c598-468d-8074-56acaf3c0453" }] }
           ]
         };
       }
 
-      if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Members?$select=displayName,mail,id`) {
+      if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Members/microsoft.graph.user?$select=displayName,mail,id&$expand=memberof($select=id),memberof($select=displayName)`) {
         return {
           "value": [
-            { "id": "00000000-0000-0000-0000-000000000001", "displayName": "Anne Matthews", "mail": "anne.matthews@contoso.onmicrosoft.com" }
+            { "id": "00000000-0000-0000-0000-000000000001", "displayName": "Anne Matthews", "mail": "anne.matthews@contoso.onmicrosoft.com", "memberOf": [{ "displayName": "Life and Music", "id": "d6c88284-c598-468d-8074-56acaf3c0454" }] }
           ]
         };
       }
 
       throw 'Invalid request';
     });
 
-    await command.action(logger, { options: { groupId: groupId, properties: "displayName,mail" } });
+    await command.action(logger, { options: { groupId: groupId, properties: "displayName,mail,memberof/id,memberof/displayName" } });
 
     assert(loggerLogSpy.calledOnceWithExactly([
-      { "id": "00000000-0000-0000-0000-000000000000", "displayName": "Karl Matteson", "mail": "karl.matteson@contoso.onmicrosoft.com", "roles": ["Owner"] },
-      { "id": "00000000-0000-0000-0000-000000000001", "displayName": "Anne Matthews", "mail": "anne.matthews@contoso.onmicrosoft.com", "roles": ["Member"] }
+      { "id": "00000000-0000-0000-0000-000000000000", "displayName": "Karl Matteson", "mail": "karl.matteson@contoso.onmicrosoft.com", "memberOf": [{ "displayName": "Life and Music", "id": "d6c88284-c598-468d-8074-56acaf3c0453" }], "roles": ["Owner"] },
+      { "id": "00000000-0000-0000-0000-000000000001", "displayName": "Anne Matthews", "mail": "anne.matthews@contoso.onmicrosoft.com", "memberOf": [{ "displayName": "Life and Music", "id": "d6c88284-c598-468d-8074-56acaf3c0454" }], "roles": ["Member"] }
     ]));
   });
 
   it('correctly lists all guest users in a Azure AD group', async () => {
     sinon.stub(request, 'get').callsFake(async (opts) => {
-      if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Owners?$select=id,displayName,userPrincipalName,givenName,surname&$filter=userType%20eq%20'Guest'&$count=true`) {
+      if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Owners/microsoft.graph.user?$select=id,displayName,userPrincipalName,givenName,surname&$filter=userType%20eq%20'Guest'&$count=true`) {
         return {
           "value": []
         };
       }
 
-      if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Members?$select=id,displayName,userPrincipalName,givenName,surname&$filter=userType%20eq%20'Guest'&$count=true`) {
+      if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Members/microsoft.graph.user?$select=id,displayName,userPrincipalName,givenName,surname&$filter=userType%20eq%20'Guest'&$count=true`) {
         return {
           "value": [
             { "id": "00000000-0000-0000-0000-000000000000", "displayName": "Anne Matthews", "userPrincipalName": "annematthews_gmail.com#EXT#@contoso.onmicrosoft.com", "givenName": "Anne", "surname": "Matthews" }
@@ -322,7 +322,7 @@ describe(commands.GROUP_USER_LIST, () => {
     };
 
     sinon.stub(request, 'get').callsFake(async (opts) => {
-      if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Owners?$select=id,displayName,userPrincipalName,givenName,surname`) {
+      if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Owners/microsoft.graph.user?$select=id,displayName,userPrincipalName,givenName,surname`) {
         throw error;
       }
 

diff --git a/src/m365/aad/commands/group/group-user-list.ts b/src/m365/aad/commands/group/group-user-list.ts
@@ -3,7 +3,6 @@ import { Logger } from '../../../../cli/Logger.js';
 import GlobalOptions from '../../../../GlobalOptions.js';
 import { CliRequestOptions } from '../../../../request.js';
 import { aadGroup } from '../../../../utils/aadGroup.js';
-import { formatting } from '../../../../utils/formatting.js';
 import { odata } from '../../../../utils/odata.js';
 import { validation } from '../../../../utils/validation.js';
 import GraphCommand from '../../../base/GraphCommand.js';
@@ -110,33 +109,26 @@ class AadGroupUserListCommand extends GraphCommand {
     try {
       const groupId = await this.getGroupId(args.options);
 
-      let users: User[] = [];
-
-      switch (args.options.role) {
-        case 'Owner':
-          users = await this.getUsers(args.options, 'Owner', groupId, logger);
-          break;
-        case 'Member':
-          users = await this.getUsers(args.options, 'Member', groupId, logger);
-          break;
-        default:
-          const owners = await this.getUsers(args.options, 'Owner', groupId, logger);
-          const members = await this.getUsers(args.options, 'Member', groupId, logger);
-
-          if (!args.options.properties) {
-            owners.forEach((owner: ExtendedUser) => {
-              for (let i = 0; i < members.length; i++) {
-                if (members[i].id === owner.id) {
-                  if (!owner.roles.includes('Member')) {
-                    owner.roles.push('Member');
-                  }
-                }
-              }
-            });
-          }
+      const users: ExtendedUser[] = [];
+
+      if (!args.options.role || args.options.role === 'Owner') {
+        const owners = await this.getUsers(args.options, 'Owners', groupId, logger);
+        owners.forEach(owner => users.push({ ...owner, roles: ['Owner'] }));
+      }
+
+      if (!args.options.role || args.options.role === 'Member') {
+        const members = await this.getUsers(args.options, 'Members', groupId, logger);
 
-          users = owners.concat(members);
-          users = users.filter((value, index, array) => index === array.findIndex(item => item.id === value.id));
+        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'] });
+          }
+        });
       }
 
       await logger.log(users);
@@ -160,13 +152,28 @@ class AadGroupUserListCommand extends GraphCommand {
     const { properties, filter } = options;
 
     if (this.verbose) {
-      await logger.logToStderr(`Retrieving ${role}s of the group with id ${groupId}`);
+      await logger.logToStderr(`Retrieving ${role} of the group with id ${groupId}`);
     }
 
     const selectProperties: string = properties ?
-      `?$select=${properties.split(',').filter(f => f.toLowerCase() !== 'id').concat('id').map(p => formatting.encodeQueryParameter(p.trim())).join(',')}` :
-      '?$select=id,displayName,userPrincipalName,givenName,surname';
-    const endpoint: string = `${this.resource}/v1.0/groups/${groupId}/${role}s${selectProperties}`;
+      `${properties.split(',').filter(f => f.toLowerCase() !== 'id').concat('id').map(p => p.trim()).join(',')}` :
+      'id,displayName,userPrincipalName,givenName,surname';
+    const allSelectProperties: string[] = selectProperties.split(',');
+    const propertiesWithSlash: string[] = allSelectProperties.filter(item => item.includes('/'));
+
+    let fieldExpand: string = '';
+    propertiesWithSlash.forEach(p => {
+      if (fieldExpand.length > 0) {
+        fieldExpand += ',';
+      }
+
+      fieldExpand += `${p.split('/')[0]}($select=${p.split('/')[1]})`;
+    });
+
+    const expandParam = fieldExpand.length > 0 ? `&$expand=${fieldExpand}` : '';
+
+    const selectParam = allSelectProperties.filter(item => !item.includes('/'));
+    const endpoint: string = `${this.resource}/v1.0/groups/${groupId}/${role}/microsoft.graph.user?$select=${selectParam}${expandParam}`;
 
     let users: ExtendedUser[] = [];
 
@@ -187,10 +194,6 @@ class AadGroupUserListCommand extends GraphCommand {
       users = await odata.getAllItems<ExtendedUser>(endpoint);
     }
 
-    users.forEach((user: ExtendedUser) => {
-      user.roles = [role];
-    });
-
     return users;
   }
 }