pnp / vscode-viva

With the SharePoint Framework Toolkit extension, you can create and manage your SharePoint Framework solutions on your tenant. All actions you need to perform during the development flow are at your fingertips.
https://marketplace.visualstudio.com/items?itemName=m365pnp.viva-connections-toolkit
MIT License
37 stars 16 forks source link

🐞 Bug report: permissions granted to the toolkit are wider than documented #302

Open michaelmaillot opened 2 days ago

michaelmaillot commented 2 days ago

⭐ Priority

(Low)☹️ Something is a little off

📝 Describe the bug

Hello,

When using the Toolkit to register (automatically) the Entra ID app which is used for the CLI, there are more API permissions granted than the ones documented.

Permissions expected to be granted according to the documentation:

https://github.com/pnp/vscode-viva/blob/main/src/webview/view/components/forms/entraAppReg/RegisterEntraAppRegView.tsx#L92

image

Permissions granted in the app:

image

Some permissions such as Directory.ReadWrite.All can be a blocker in some organizations.

👣 Steps To Reproduce

📜 Expected behavior

Permissions granted in the documentation should reflect the effective ones.

📷 Screenshots

No response

❓SharePoint Framework Toolkit version

4.0.0

❓Node.js version

18.19.0

🤔 Additional context

No response

Adam-it commented 2 days ago

@michaelmaillot thanks for pointing it out. I double checked and in code we actually set

 'https://graph.windows.net/Directory.AccessAsUser.All',
  'https://management.azure.com/user_impersonation',
  'https://graph.microsoft.com/User.Read',
  'https://graph.microsoft.com/AppCatalog.ReadWrite.All',
  'https://graph.microsoft.com/AuditLog.Read.All',
  'https://graph.microsoft.com/SecurityEvents.Read.All',
  'https://graph.microsoft.com/ServiceHealth.Read.All',
  'https://graph.microsoft.com/ServiceMessage.Read.All',
  'https://graph.microsoft.com/Sites.Read.All',
  'https://graph.microsoft.com/Directory.AccessAsUser.All',
  'https://graph.microsoft.com/Directory.ReadWrite.All',
  'https://manage.office.com/ActivityFeed.Read',
  'https://manage.office.com/ServiceHealth.Read',
  'https://microsoft.sharepoint-df.com/AllSites.FullControl',
  'https://microsoft.sharepoint-df.com/User.ReadWrite.All'

it seems in documentation I missed adding info about 🤦‍♂️

  'https://graph.windows.net/Directory.AccessAsUser.All',
  'https://management.azure.com/user_impersonation',

I will need to include it in the documentation. Thank you for pointing it out. You Rock 🤩

michaelmaillot commented 2 days ago

If I'm not mistaken, there are also the following ones which are missing in the docs:

'https://graph.microsoft.com/Directory.AccessAsUser.All',
'https://graph.microsoft.com/Directory.ReadWrite.All'

Regarding the "Register Entra App" page in the extension:

image

Adam-it commented 2 days ago

yes, that as well 👍. Thanks for the double check

Adam-it commented 2 days ago

@michaelmaillot would you like to take it up and contribute to this repo? Or you would rather leave it for me? if needed I may point you in the places where we need to correct it.