-
Notifications
You must be signed in to change notification settings - Fork 351
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
Comments
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 ? |
@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! 😉 |
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. |
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 |
That's great, @KoenZomers. |
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. |
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.
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.
The text was updated successfully, but these errors were encountered: