[ap0phi5]

Overview

This is a small change to reduce the default permission scope for a Microsoft connector from the overly-permissive directory.read.all down to groupMember.read.all.

What this PR does / why we need it

Closes #3989

[ap0phi5]

Anyone able to allow this to run the CI tests?

[ap0phi5]

@nabokihms If it is within your purview, could you let the tests run on this please?
The details look a bit old but I'm assuming this tenant is still available?
https://github.com/dexidp/dex/blob/master/connector/microsoft/microsoft_test.go#L20-L23

[ap0phi5]

Bump. Is there anything more I can do to help this PR be considered?

[cardoe]

Reading through what is needed @ap0phi5 is correct that the lesser permission is good enough.

[sagikazarmark]

Looks like CI is stuck. In any case, I wonder if it would make sense to make this configurable. @cardoe is telling me that it requires reapproval which may be a problem for users.

We could leave the default the old value for a while and after a grace period change it to the new one.

[sagikazarmark]

Actually, I've been thinking about something like Kubernetes' feature flag for a while now. This would be a prime candidate.

[Geethree]

It would be really great to get this merged!

[lukeplausin]

It's been quite a long time now since the issue was raised and no action taken.

Personally I think it's very little effort for an Entra SysAdmin to approve a permissions change request for apps that are reducing their permissions from Directory.Read.All to GroupMember.Read.All, and something most admins would be happy to do.

@sagikazarmark would you be happy to approve the change without configuration, on the basis that it's a security improvement that most Entra SysAdmins would welcome?