Skip to content

Commit 4fbb54d

Browse files
nanddeepnnanddeepn
committed
review comments
1 parent 20c9beb commit 4fbb54d

File tree

3 files changed

+39
-78
lines changed

3 files changed

+39
-78
lines changed

‎docs/docs/cmd/aad/group/group-user-list.mdx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,14 @@ m365 aad group user list [options]
2323

2424
`-r, --role [role]`
2525
: Filter the results to only users with the given role: `Owner`, `Member`.
26-
```
2726

2827
`-p, --properties [properties]`
2928
: Comma-separated list of properties to retrieve.
3029

3130
`-f, --filter [filter]`
3231
: OData filter to use to query the list of users with.
32+
```
33+
3334
<Global />
3435

3536
## Examples
@@ -117,3 +118,8 @@ m365 aad group user list --groupDisplayName Developers --filter "userType eq 'Gu
117118

118119
</TabItem>
119120
</Tabs>
121+
122+
123+
## More information
124+
125+
- View the documentation to see what userproperties can be included: [https://pnp.github.io/cli-microsoft365/cmd/aad/user/user-get#more-information](https://pnp.github.io/cli-microsoft365/cmd/aad/user/user-get#more-information)

‎src/m365/aad/commands/group/group-user-list.spec.ts

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -103,13 +103,13 @@ describe(commands.GROUP_USER_LIST, () => {
103103

104104
it('correctly lists all users in a Azure AD group by id', async () => {
105105
sinon.stub(request, 'get').callsFake(async (opts) => {
106-
if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/owners?$select=id,displayName,userPrincipalName,givenName,surname`) {
106+
if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Owners?$select=id,displayName,userPrincipalName,givenName,surname`) {
107107
return {
108108
"value": [{ "id": "00000000-0000-0000-0000-000000000000", "displayName": "Anne Matthews", "userPrincipalName": "anne.matthews@contoso.onmicrosoft.com", "givenName": "Anne", "surname": "Matthews" }]
109109
};
110110
}
111111

112-
if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/members?$select=id,displayName,userPrincipalName,givenName,surname`) {
112+
if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Members?$select=id,displayName,userPrincipalName,givenName,surname`) {
113113
return {
114114
"value": [
115115
{ "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, () => {
147147
sinon.stub(aadGroup, 'getGroupIdByDisplayName').resolves(groupId);
148148

149149
sinon.stub(request, 'get').callsFake(async (opts) => {
150-
if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/owners?$select=id,displayName,userPrincipalName,givenName,surname`) {
150+
if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Owners?$select=id,displayName,userPrincipalName,givenName,surname`) {
151151
return {
152152
"value": [{ "id": "00000000-0000-0000-0000-000000000000", "displayName": "Anne Matthews", "userPrincipalName": "anne.matthews@contoso.onmicrosoft.com", "givenName": "Anne", "surname": "Matthews" }]
153153
};
154154
}
155155

156-
if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/members?$select=id,displayName,userPrincipalName,givenName,surname`) {
156+
if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Members?$select=id,displayName,userPrincipalName,givenName,surname`) {
157157
return {
158158
"value": [
159159
{ "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, () => {
189189

190190
it('correctly lists all owners in a Azure AD group', async () => {
191191
sinon.stub(request, 'get').callsFake(async (opts) => {
192-
if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/owners?$select=id,displayName,userPrincipalName,givenName,surname`) {
192+
if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Owners?$select=id,displayName,userPrincipalName,givenName,surname`) {
193193
return {
194194
"value": [{ "id": "00000000-0000-0000-0000-000000000000", "displayName": "Anne Matthews", "userPrincipalName": "anne.matthews@contoso.onmicrosoft.com", "givenName": "Anne", "surname": "Matthews" }]
195195
};
@@ -212,7 +212,7 @@ describe(commands.GROUP_USER_LIST, () => {
212212

213213
it('correctly lists all members in a Azure AD group', async () => {
214214
sinon.stub(request, 'get').callsFake(async (opts) => {
215-
if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/members?$select=id,displayName,userPrincipalName,givenName,surname`) {
215+
if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Members?$select=id,displayName,userPrincipalName,givenName,surname`) {
216216
return {
217217
"value": [
218218
{ "id": "00000000-0000-0000-0000-000000000000", "displayName": "Anne Matthews", "userPrincipalName": "anne.matthews@contoso.onmicrosoft.com", "givenName": "Anne", "surname": "Matthews" },
@@ -248,18 +248,18 @@ describe(commands.GROUP_USER_LIST, () => {
248248

249249
it('correctly lists properties for all users in a Azure AD group', async () => {
250250
sinon.stub(request, 'get').callsFake(async (opts) => {
251-
if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/owners?$select=displayName,mail`) {
251+
if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Owners?$select=displayName,mail,id`) {
252252
return {
253253
"value": [
254-
{ "displayName": "Karl Matteson", "mail": "karl.matteson@contoso.onmicrosoft.com" }
254+
{ "id": "00000000-0000-0000-0000-000000000000", "displayName": "Karl Matteson", "mail": "karl.matteson@contoso.onmicrosoft.com" }
255255
]
256256
};
257257
}
258258

259-
if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/members?$select=displayName,mail`) {
259+
if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Members?$select=displayName,mail,id`) {
260260
return {
261261
"value": [
262-
{ "displayName": "Anne Matthews", "mail": "anne.matthews@contoso.onmicrosoft.com" }
262+
{ "id": "00000000-0000-0000-0000-000000000001", "displayName": "Anne Matthews", "mail": "anne.matthews@contoso.onmicrosoft.com" }
263263
]
264264
};
265265
}
@@ -270,20 +270,20 @@ describe(commands.GROUP_USER_LIST, () => {
270270
await command.action(logger, { options: { groupId: groupId, properties: "displayName,mail" } });
271271

272272
assert(loggerLogSpy.calledOnceWithExactly([
273-
{ "displayName": "Karl Matteson", "mail": "karl.matteson@contoso.onmicrosoft.com" },
274-
{ "displayName": "Anne Matthews", "mail": "anne.matthews@contoso.onmicrosoft.com" }
273+
{ "id": "00000000-0000-0000-0000-000000000000", "displayName": "Karl Matteson", "mail": "karl.matteson@contoso.onmicrosoft.com", "roles": ["Owner"] },
274+
{ "id": "00000000-0000-0000-0000-000000000001", "displayName": "Anne Matthews", "mail": "anne.matthews@contoso.onmicrosoft.com", "roles": ["Member"] }
275275
]));
276276
});
277277

278278
it('correctly lists all guest users in a Azure AD group', async () => {
279279
sinon.stub(request, 'get').callsFake(async (opts) => {
280-
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`) {
280+
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`) {
281281
return {
282282
"value": []
283283
};
284284
}
285285

286-
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`) {
286+
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`) {
287287
return {
288288
"value": [
289289
{ "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, () => {
322322
};
323323

324324
sinon.stub(request, 'get').callsFake(async (opts) => {
325-
if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/owners?$select=id,displayName,userPrincipalName,givenName,surname`) {
325+
if (opts.url === `https://graph.microsoft.com/v1.0/groups/2c1ba4c4-cd9b-4417-832f-92a34bc34b2a/Owners?$select=id,displayName,userPrincipalName,givenName,surname`) {
326326
throw error;
327327
}
328328

‎src/m365/aad/commands/group/group-user-list.ts

Lines changed: 17 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -114,19 +114,19 @@ class AadGroupUserListCommand extends GraphCommand {
114114

115115
switch (args.options.role) {
116116
case 'Owner':
117-
users = await this.getOwners(args.options, groupId, logger);
117+
users = await this.getUsers(args.options, 'Owner', groupId, logger);
118118
break;
119119
case 'Member':
120-
users = await this.getMembers(args.options, groupId, logger);
120+
users = await this.getUsers(args.options, 'Member', groupId, logger);
121121
break;
122122
default:
123-
const owners = await this.getOwners(args.options, groupId, logger);
124-
const members = await this.getMembers(args.options, groupId, logger);
123+
const owners = await this.getUsers(args.options, 'Owner', groupId, logger);
124+
const members = await this.getUsers(args.options, 'Member', groupId, logger);
125125

126126
if (!args.options.properties) {
127127
owners.forEach((owner: ExtendedUser) => {
128128
for (let i = 0; i < members.length; i++) {
129-
if (members[i].userPrincipalName === owner.userPrincipalName) {
129+
if (members[i].id === owner.id) {
130130
if (!owner.roles.includes('Member')) {
131131
owner.roles.push('Member');
132132
}
@@ -136,10 +136,7 @@ class AadGroupUserListCommand extends GraphCommand {
136136
}
137137

138138
users = owners.concat(members);
139-
140-
if (!args.options.properties) {
141-
users = users.filter((value, index, array) => index === array.findIndex(item => item.userPrincipalName === value.userPrincipalName));
142-
}
139+
users = users.filter((value, index, array) => index === array.findIndex(item => item.id === value.id));
143140
}
144141

145142
await logger.log(users);
@@ -159,19 +156,19 @@ class AadGroupUserListCommand extends GraphCommand {
159156
return await aadGroup.getGroupIdByDisplayName(groupDisplayName!);
160157
}
161158

162-
private async getOwners(options: Options, groupId: string, logger: Logger): Promise<ExtendedUser[]> {
159+
private async getUsers(options: Options, role: string, groupId: string, logger: Logger): Promise<ExtendedUser[]> {
163160
const { properties, filter } = options;
164161

165162
if (this.verbose) {
166-
await logger.logToStderr(`Retrieving owners of the group with id ${groupId}`);
163+
await logger.logToStderr(`Retrieving ${role}s of the group with id ${groupId}`);
167164
}
168165

169166
const selectProperties: string = properties ?
170-
`?$select=${properties.split(',').map(p => formatting.encodeQueryParameter(p.trim())).join(',')}` :
167+
`?$select=${properties.split(',').filter(f => f.toLowerCase() !== 'id').concat('id').map(p => formatting.encodeQueryParameter(p.trim())).join(',')}` :
171168
'?$select=id,displayName,userPrincipalName,givenName,surname';
172-
const endpoint: string = `${this.resource}/v1.0/groups/${groupId}/owners${selectProperties}`;
169+
const endpoint: string = `${this.resource}/v1.0/groups/${groupId}/${role}s${selectProperties}`;
173170

174-
let owners: ExtendedUser[] = [];
171+
let users: ExtendedUser[] = [];
175172

176173
if (filter) {
177174
// While using the filter, we need to specify the ConsistencyLevel header.
@@ -184,59 +181,17 @@ class AadGroupUserListCommand extends GraphCommand {
184181
responseType: 'json'
185182
};
186183

187-
owners = await odata.getAllItems<ExtendedUser>(requestOptions);
184+
users = await odata.getAllItems<ExtendedUser>(requestOptions);
188185
}
189186
else {
190-
owners = await odata.getAllItems<ExtendedUser>(endpoint);
191-
}
192-
193-
if (!properties) {
194-
owners.forEach((user: ExtendedUser) => {
195-
user.roles = ['Owner'];
196-
});
197-
}
198-
199-
return owners;
200-
}
201-
202-
private async getMembers(options: Options, groupId: string, logger: Logger): Promise<ExtendedUser[]> {
203-
const { properties, filter } = options;
204-
205-
if (this.verbose) {
206-
await logger.logToStderr(`Retrieving members of the group with id ${groupId}`);
187+
users = await odata.getAllItems<ExtendedUser>(endpoint);
207188
}
208189

209-
const selectProperties: string = properties ?
210-
`?$select=${properties.split(',').map(p => formatting.encodeQueryParameter(p.trim())).join(',')}` :
211-
'?$select=id,displayName,userPrincipalName,givenName,surname';
212-
const endpoint: string = `${this.resource}/v1.0/groups/${groupId}/members${selectProperties}`;
213-
214-
let members: ExtendedUser[] = [];
215-
216-
if (filter) {
217-
// While using the filter, we need to specify the ConsistencyLevel header.
218-
const requestOptions: CliRequestOptions = {
219-
url: `${endpoint}&$filter=${encodeURIComponent(filter)}&$count=true`,
220-
headers: {
221-
accept: 'application/json;odata.metadata=none',
222-
ConsistencyLevel: 'eventual'
223-
},
224-
responseType: 'json'
225-
};
226-
227-
members = await odata.getAllItems<ExtendedUser>(requestOptions);
228-
}
229-
else {
230-
members = await odata.getAllItems<ExtendedUser>(endpoint);
231-
}
232-
233-
if (!properties) {
234-
members.forEach((user: ExtendedUser) => {
235-
user.roles = ['Member'];
236-
});
237-
}
190+
users.forEach((user: ExtendedUser) => {
191+
user.roles = [role];
192+
});
238193

239-
return members;
194+
return users;
240195
}
241196
}
242197

0 commit comments

Comments
 (0)