[BUG] Using groupId property of PeoplePicker incorrectly requires User.Read.All scope

[NathZ1]

Describe the bug It appears that the PeoplePicker component is configured to require User.Read.All scope (in addition to People.Read and GroupMember.Read.All) but the Graph documentation for /groups/${groupId}/members specifies (from least to most privileged): GroupMember.Read.All, Group.Read.All, GroupMember.ReadWrite.All, Group.ReadWrite.All, Directory.Read.All. There is no requirement for User.Read.All permission.

I tested with an application with People.Read and GroupMember.Read.All scopes granted (without granting User.Read.All) and disabling incremental consent, and the PeoplePicker component worked as expected when passed a groupId, filtering the results perfectly.

Would it be possible to update the required scopes of PeoplePicker component to not require User.Read.All scope (and associated documentation) please?

To Reproduce Steps to reproduce the behavior:

1. Grant scopes People.Read and GroupMember.Read.All only
2. Disable incremental consent in provider config
3. Add People Picker component and pass groupId property of a known group

Expected behavior When using groupId property, PeoplePicker component works without User.Read.All scope granted

Environment (please complete the following information):

• OS: Windows 10
• Browser: Chromium
• Framework: React
• Context:
• Version: 2.6.1
• Provider: MSAL2

[msftbot[bot]]

Hello NathZ1, thank you for opening an issue with us!

I have automatically added a "needs triage" label to help get things started. Our team will analyze and investigate the issue, and escalate it to the relevant team if possible. Other community members may also look into the issue and provide feedback 🙌

[sebastienlevert]

Thanks @NathZ1! This is an interesting topic as I would assume User.Read.All would be required for other picker scenarios. Your expectations is when using a groupId, we should avoid requesting this scope, correct?

@gavinbarron I'd love to discuss this one more as I think it's a valid scenario, especially in a zero trust mindset. I don't know if our components can be fine grained at that level, but we should think about it! This would probably becomes a component-wide permissions approach that needs to be reviewed, inspired by the uses cases for this issue!

[NathZ1]

Hi @sebastienlevert, you are correct - there seems to be no need to require this scope, as the graph query works fine without it. Additionally, looking through the People Picker permission documention, it appears that User.Read.All is only stated as needed when group-id is set, with other configurations only requiring User.ReadBasic.All -

https://learn.microsoft.com/en-us/graph/toolkit/components/people-picker

I suspect User.Read.All is not actually required at all for People Picker component.

Cheers Nathan

[gavinbarron]

For the people picker directly, no it's not needed AFAIK. However, to render the selected people we user the mgt-person which in turn uses the mgt-person-card which on the organization pane requires the User.Read.All scope: https://learn.microsoft.com/en-us/graph/toolkit/components/person-card#microsoft-graph-permissions

So yes, in the scenario of showing Groups that scope is not necessary, the current implementation is to try and request all the scopes that are needed for the control when it renders.

I think we should explore making the scopes being requested more dynamic based upon the inputs given for a control.