pnp / cli-microsoft365

Manage Microsoft 365 and SharePoint Framework projects on any platform
https://aka.ms/cli-m365
MIT License
910 stars 317 forks source link

New command: user membership (transitive and direct) #4461

Open chlunde opened 1 year ago

chlunde commented 1 year ago

I think it is best if you take a look at what arguments are reasonable here. You should also consider if transitive membership and direct membership are two different commands or an option to a membership command, or if they both are options to get.

The REST API is documented here: https://learn.microsoft.com/en-us/graph/api/user-list-transitivememberof?view=graph-rest-1.0&tabs=http


Usage

m365 user transitiveMemberOf

Description

Retrieve transitive groups for user

Options

Similar options as user --get?

Option Description
-i, --teamId <teamId> The Id of the Microsoft Team
-r, --role [role] Filter the results to only users with the given role: Owner or Member.

Examples

TODO: Edit me

m365 teams channel add --teamName "Team Name" --name climicrosoft365 --description development

Default properties

Additional Info

No response

milanholemans commented 1 year ago

Hi @chlunde

First of all thanks for this suggestion! Seems like this is indeed not present in the CLI yet. If I take a look at the API, it seems like this is somewhat related to https://learn.microsoft.com/en-us/graph/api/user-list-memberof?view=graph-rest-1.0&tabs=http.

Regarding the options, I think we don't need any options if we use delegated authentication right? I suggest we do the following:

Option Description
--userName [userName] User's UPN (user principal name, e.g. johndoe@example.com). Specify either userName or userId but not both.
--userId [userId] User's Azure AD Id. Specify either userName or userId but not both.
--transitive Include transitive memberships.

When --transitive is not specified, we use the /memberOf endpoint, if it is specified, we use the /transitiveMemberOf endpoint.

Is this something that works for you @chlunde?

martinlingstuyl commented 1 year ago

Great idea @chlunde and I agree on @milanholemans options. Slight edit: I'd rename the transitive option and its description, I think it's clearer to the end user if we name it something like --includeNested. Eg: use the word nested instead of transitive.

Should these be added to a new command called m365 aad user group list? Or do you have different ideas?

milanholemans commented 1 year ago

Great idea @chlunde and I agree on @milanholemans options. Slight edit: I'd rename the transitive option and its description, I think it's clearer to the end user if we name it something like --includeNested. Eg: use the word nested instead of transitive.

Thinking of it again, for me --indirect or --indirectMembership something like that would be clearer.

Should these be added to a new command called m365 aad user group list? Or do you have different ideas?

m365 aad user group list sounds good I guess!

martinlingstuyl commented 1 year ago

indirect sounds good as well. Question: what's more logical: include indirect membership by default and let users exclude it? Or the other way around?

@pnp/cli-for-microsoft-365-maintainers any thoughts about this command?

milanholemans commented 1 year ago

indirect sounds good as well. Question: what's more logical: include indirect membership by default and let users exclude it? Or the other way around?

Was thinking the same actually. I like the idea to include transitive membership by default (which is most convenient I guess) and add a flag --directMembership or something like that.

martinlingstuyl commented 1 year ago

How should we call that flag option? onlyDirectMembership? excludeNested?

Personally I'd veer towards the latter, as it's clearer in my eyes. Direct is still a bit vague, while nested gives me a sense of depth.

milanholemans commented 1 year ago

I'm not really a fan of the nested word here, but that might be just me. In both Graph docs they make never use of the word nested.

martinlingstuyl commented 1 year ago

Whether a term is used in a technical api is no reason to use it or not use it I think.

But thinking about it, maybe --onlyDirectMemberships is clear.

Any other @pnp/cli-for-microsoft-365-maintainers?

Let's update the specs so that can have a proper peer review.