microsoftgraph / microsoft-graph-toolkit

Authentication Providers and UI components for Microsoft Graph 🦒
https://docs.microsoft.com/graph/toolkit/overview
Other
949 stars 305 forks source link

Change PeoplePicker defaultSelectedGroupIds to use "/groups" route instead of "/directoryObjects" #2793

Closed sscots closed 1 year ago

sscots commented 1 year ago

Proposal: Change PeoplePicker defaultSelectedGroupIds to use "/groups" route instead of "/directoryObjects"

Description

We are using the PeoplePicker to allow users to select groups to associate them to our own internal entity. When we go to edit a entity, we set "defaultSelectedGroupIds" to the one we have stored for that entity. Currently we have Group.Read.All access allowed for the Azure App Registration but not Directory.Read.All. Our corporation does not allows Directory.Read.All because it's too broad of an access.

Rationale

1) Directory.Read.All is too broad of an access level 2) defaultSelectedGroupIds is very specific to the /groups route and therefor makes more sense to use instead

Preferred Solution

An example of getting a list from defaultSelectedGroupIds /groups?$filter=id eq 'c81905b4-3912-4ab3-88ef-85c362a6f568' or id eq '7dc8d02b-0848-4b33-ace2-e9a2e40a9a29'

sebastienlevert commented 1 year ago

Thanks for reporting your issue @sscots. Can you expand on when you see this? We don't use /directoryObjects in MGT when looking up groups as seen here : https://github.com/microsoftgraph/microsoft-graph-toolkit/blob/main/packages/mgt-components/src/graph/graph.groups.ts

If you can provide a repro, this would be great! Thanks!

sscots commented 1 year ago

@sebastienlevert I can work on a repro. But it only happens whenever I set "defaultSelectedGroupIds". It appears that when I set defaultSelectedGroupIds to a list of group ids, that attribute triggers the /directoryObject route to obtain the name of those default groups in order to just display the names in the input field. If I remove "defaultSelectedGroupIds" or set it to an empty list, it works fine and the /directoryObjects route is never called.

sebastienlevert commented 1 year ago

A repro would be nice. Currently, when looking at our version of the Toolkit, I can't find any reference to /directoryObjects so I'm a bit confused ;) What is the version you are using?

sscots commented 1 year ago

I should mention this is also using @microsoft/mgt-react

sebastienlevert commented 1 year ago

Using the React wrappers shouldn't make a difference here as it basically just creates React components from the Web Components we provide, so the same code!