Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New command: aad group user list #5469

Closed
Tracked by #5432
milanholemans opened this issue Sep 7, 2023 · 15 comments
Closed
Tracked by #5432

New command: aad group user list #5469

milanholemans opened this issue Sep 7, 2023 · 15 comments
Assignees
Labels
Milestone

Comments

@milanholemans
Copy link
Contributor

milanholemans commented Sep 7, 2023

New specs

We've created new specs because there was a confusion with the old specs around member roles and user types. userType (Members VS Guests) is an AAD related user property, while Owners VS Members is the role the user has in the group.

The changes specs:

Usage

m365 aad group user list [options]

Description

Lists users of a specific Azure AD group

Options

Option Description
-i, --groupId [groupId] The ID of the Azure AD group. Specify groupId or groupDisplayName but not both.
-n, --groupDisplayName [groupDisplayName] The display name of the Azure AD group. Specify groupId or groupDisplayName but not both.
-r, --role [role] Filter the results to only users with the given role: Owner or Member.
-p, --properties [properties] Comma-separated list of properties to retrieve.
-f, --filter [filter] OData filter to use to query the list of users with.

Examples

List all group users from a group specified by ID

m365 aad group user list --groupId 03cba9da-3974-46c1-afaf-79caa2e45bbe

List all owners from a group specified by display name

m365 aad group user list --groupDisplayName Developers --role Owner

List specific properties for all group users from a group specified by ID

m365 aad group user list --groupId 03cba9da-3974-46c1-afaf-79caa2e45bbe --properties 'id,jobTitle,companyName,accountEnabled'

List all group members that are guest users

m365 aad group user list --groupDisplayName Developers --filter "userType eq 'Guest'"

Default properties

  • id
  • displayName
  • userPrincipalName
  • roles (custom property)

Additional Info

  • in the api call we $select the userName, id, displayName, given name and surname by default.

  • When using the --properties option, default properties will not be added to the $select parameter, unless mentioned

  • Just like in similar commands, using a property with a slash should translate to an added $expand-parameter.

  • we add a roles property to the items in the response output to show that users are member, owner or both.

More information

View the documentation to see what userproperties can be included: https://pnp.github.io/cli-microsoft365/cmd/aad/user/user-get#more-information

Old Specs

Usage

m365 aad group user list [options]

Description

Lists users of a specific Azure AD group

Options

Option Description
-i, --groupId [groupId] The ID of the Azure AD group. Specify groupId or groupDisplayName but not both.
-n, --groupDisplayName [groupDisplayName] The display name of the Azure AD group. Specify groupId or groupDisplayName but not both.
-r, --role [role] Filter the results to only users with the given role: Owner, Member or Guest.

Examples

List all group users from a group specified by ID

m365 aad group user list --groupId 03cba9da-3974-46c1-afaf-79caa2e45bbe

List all owners from a group specified by display name

m365 aad group user list --groupDisplayName Developers --role Owner

Default properties

  • id
  • displayName
  • userPrincipalName
  • userType

Additional Info

  • In the API we should explicitly select the property userType otherwise it will not be returned.
@Jwaegebaert
Copy link
Contributor

LGTM! Let's open it up.

@Jwaegebaert Jwaegebaert added help wanted and removed needs peer review Needs second pair of eyes to review the spec or PR labels Sep 8, 2023
@nanddeepn
Copy link
Contributor

Can I work on this?

@nanddeepn
Copy link
Contributor

Hi @milanholemans,

Just wanted to check, how this command is different from aad m365group user list?
In the aad m365group user list we are not limiting that the group specified should be unified. So, the same implementation will work for this command too?

Please correct, if I missed something.

@milanholemans
Copy link
Contributor Author

Hi @nanddeepn

Yes, currently it is possible to run m365group user list for security groups for example. But this is really not ideal. People don't expect to use this command for a security group.
Therefore we created issue #5438 to ensure that m365group commands only work for unified groups.

The aad group user list won't have this limit and should work for all groups.

@martinlingstuyl
Copy link
Contributor

I've updated the specs @milanholemans, @nanddeepn l, @pnp/cli-for-microsoft-365-maintainers. Thoughts?

@milanholemans
Copy link
Contributor Author

we add a role property to the items in the response output to show that users are member, owner or both.

So this means that it looks something like this?

{
  "role": [ "Owner", "Member"]
}

In that case, I'd call it roles (plural).

Let's also add a link to the page where all possible User properties are listed like we do here: https://pnp.github.io/cli-microsoft365/cmd/aad/user/user-get#more-information

@martinlingstuyl
Copy link
Contributor

@nanddeepn, we've reached a conclusion on the changed specs. Could you take this for a spin and update your PR? Do you have questions? Please let us know!

@nanddeepn
Copy link
Contributor

Thank you @martinlingstuyl
Specs are clear to me. I will resume the work on this PR.

@nanddeepn
Copy link
Contributor

Hi @martinlingstuyl
To make the filter work here, we need to set the ConsistencyLevel:eventual header.

Similar discussion, we have going over here: #5424

@martinlingstuyl
Copy link
Contributor

Hi @nanddeepn, what's the reason the extra header is necessary? Have you researched that?

@nanddeepn
Copy link
Contributor

Hi @martinlingstuyl

The filter query does not work
image

It does work after specifying the header.
image

Reference:
https://learn.microsoft.com/en-us/answers/questions/464665/graph-api-with-filter-ref-properties-does-not-seem

@martinlingstuyl
Copy link
Contributor

Ok, i suggest we use the overloads idea that I've pitched in the other comment here as well. It may take some syncing between you and @SmitaNachan 😂. If you both commit the same code, we may get a merge conflict, but that's okay, we'll figure it out.

Please do write this explanation in a comment above the line where you use the overload. We may refactor it back later, when the header is no longer needed here...

@martinlingstuyl
Copy link
Contributor

martinlingstuyl commented Oct 11, 2023

@nanddeepn, @milanholemans, I've added an extra bullet below additional information:

  • When using the --properties option, default properties will not be added to the $select parameter, unless mentioned

Does this make sense to you?

I also added an extra example

@milanholemans
Copy link
Contributor Author

I think it would make sense that only the provided properties would be retrieved; so, this explanation is unnecessary. But I can live with it 🙂

@martinlingstuyl
Copy link
Contributor

I agree it makes sense. But for clarifications sake! It will break the text output view, all would be empty. But that's okay.

@martinlingstuyl martinlingstuyl added hacktoberfest Issue perfect for hacktoberfest and removed work in progress labels Oct 25, 2023
@martinlingstuyl martinlingstuyl added this to the v7.1 milestone Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants