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

Add required permissions to each cmdlet #4353

Open
sympmarc opened this issue Sep 27, 2024 · 7 comments
Open

Add required permissions to each cmdlet #4353

sympmarc opened this issue Sep 27, 2024 · 7 comments
Labels
documentation Improvements or additions to documentation enhancement New feature or request help wanted Extra attention is needed

Comments

@sympmarc
Copy link
Contributor

With the recent changes to the app registration for the module, it would be great to have a consistent bit of info on each cmdlet showing which permissions are required for it to work. Some cmdlets have this already, like Get-PnPAzureADAppSitePermission. Others, like Get-PnPSite do not.

image

image

If there's a way to generate a list of cmdlets which don't have this info, we could crowdsource the content.

Understanding what permissions are required to accomplish specific things can be problematic, as shown in issue #4351. While it might seem anyone running PnP.PowerShell should inherently know which permissions are required, I'm positive that's not the case. It certainly isn't obvious to me most of the time, now that I'm having to think about it with the recent changes. In many cases, we have to ask someone else to consent to the permissions (totally reasonable) and it is important we know which permissions to ask for.

@sympmarc sympmarc added the enhancement New feature or request label Sep 27, 2024
@jackpoz
Copy link
Contributor

jackpoz commented Sep 28, 2024

It's worth to mention that cmdlet parameters affect which permissions are needed.

Take https://pnp.github.io/powershell/cmdlets/Set-PnPPlannerTask.html as example, with permissions "Microsoft Graph API: One of Tasks.ReadWrite, Tasks.ReadWrite.All, Group.ReadWrite.All". When using the "-AssignedTo" parameter, then "User.Read.All" is also needed, which led to #4310 .

Maybe these additional permissions should be listed in the details of each parameter ? Or maybe this information fits better on top in the existing "Required Permissions" but in a different way to show that, if you don't use all parameters, you don't need all permissions ?

@veronicageek
Copy link
Collaborator

veronicageek commented Sep 28, 2024

@sympmarc - That's a very valid ask, and we will work on that. Many cmdlets using Graph (i.e.: Teams, Planner, EntraID) have already good documentation around the Graph permissions but also appreciate what @jackpoz is saying. SharePoint cmdlets use mostly SPO API at this time (not Graph), so... lots of work in perspective.

And of course, anyone can jump in! 😉

@veronicageek veronicageek added help wanted Extra attention is needed documentation Improvements or additions to documentation labels Sep 28, 2024
@veronicageek veronicageek changed the title [DOCS] Add required permissions to each cmdlet Add required permissions to each cmdlet Sep 28, 2024
@sympmarc
Copy link
Contributor Author

sympmarc commented Sep 30, 2024

If there are "groupings" of cmdlets, like many of the SharePoint ones, would it make more sense to have a separate page we can link to which explains the permissions for the grouping?

I think @jackpoz's idea of listing the permissions required for specific parameter makes sense, too. I'll bet a lot of the issues you get come down to permissions, like the one @jackpoz highlighted.

I think if we could identify the holes, we can make the docs better by doing this. I'm happy to help, and I'll bet I can rope @ToddKlindt in, too.

@KoenZomers
Copy link
Collaborator

In this context, I have performed work to perform permission validation during execution to make things even easier. Just take the latest nightly, run cmdlets with -Verbose and you will see exactly which Graph API is being called into and if the permissions for it are already known. If not, PRs are very much welcomed where these will be added. Please refer to this documentation on how to add them:

https://pnp.github.io/powershell/articles/permissionattributes.html

@sympmarc
Copy link
Contributor Author

sympmarc commented Oct 7, 2024

That's great, @KoenZomers.

@wilecoyotegenius
Copy link
Contributor

And how do we handle Sites,Selected?

@KoenZomers
Copy link
Collaborator

And how do we handle Sites,Selected?

Just by adding it as an option to the attributes. We're not going to live check the actual permissions for that though. It would cause way too much unneccessary load and delays.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants