microsoftgraph
Add support for unconsenting a permission
Is your feature request related to a problem? Please describe. It is currently not possible from within Graph Explorer to remove a previously consented scope to test scopes with lower privileges.
@RabebOthmani this work item needs a UI/UX design wireframe or the other option if for us to prototype and review together
@adhiambovivian my plan is to do minimal UI/UX changes due to resources. We will rely on the current UI and make necessary changes on it.
@darrelmiller @adhiambovivian @MaryannGitonga @gavinbarron for the permissions tab, here's what I'm thinking. Please remember that the goal here is not to reinvent the UI and do minimum required work there.
This is the current tab:
I'm thinking:
so it would look something like this
@MaryannGitonga as for the actual implementation, what we need is to update oAuth2PermissionGrant to remove items (permissions in this case). Forgot the link doh https://docs.microsoft.com/en-us/graph/api/oauth2permissiongrant-delete?view=graph-rest-1.0&tabs=http @adhiambovivian I'd suggest pairing Maryann with Evans or Elinor. Permissions are tricky to understand, let us give her the support she needs :)
@darrelmiller @gavinbarron @adhiambovivian @MaryannGitonga for the Permissions list under the user's profile. I captured some challenges with the current experience
https://user-images.githubusercontent.com/5781590/184618287-4ade6cf7-3ee1-4f64-b12c-f7e2ebff9496.mp4
Similar to the permissions tab, is there any value of showing a count of the permissions selected?
After some thought, do we want to encourage users to select a group of permissions? Isn't that against the recommendation to consent the least privileged whenever possible? @darrelmiller I'd love your opinion on this. My suggestions would be to remove the checkboxes, keep the list grouping and apply similar modifications to what we will apply on permissions tab (consent/unconsent button , yes/no admin etc)
How are we filtering the list of permissions cc @adhiambovivian
Family permission is hidden on Graph but it appears on the list. It shouldn't
While working on this, we noted some interesting things: GE requires the User.Read permission to make bare-minimum calls. What do we do in case a user unconsents to User.Read permission? Do we log them out? Do we restrict the ability to consent to such a permission?
Unconsenting requires the following permissions:
These permissions require admin consent before a user can consent to them. What do we do in such a scenario?
I'm extrapolating a little here but it sounds like at an absolute minimum the user needs to have User.Read for some calls and there is no lower valid permission that can be consented to. This is coupled with the fact that to unconsent to a permission Directory.Read.All is required.
Can we prevent these permissions from being unconsented and add a call out in the UI that User.Read and Directory.Read.All are the lowest permissions available to still have the GE application functional?
@gavinbarron in that case, should Directory.Read.All (and DelegatedPermissionGrant.ReadWrite.All which is another permission required to enable unconsenting to permissions) be added to the list of default scopes that a user is consented to first off, on signing into GE?
Great question, and I'm sorry to answer with another question. What are the scopes we request on the initial sign-in at present? Are there permissions that require admin consent for a user to sign in? (My gut is that we do need admin consent being a multi-tenant app).
I'm hesitant to ask for those permissions until we absolutely need them, such as when a user wants to revoke consent for a scope just due to them having such wide reach.
As a good practice we should ask only for the minimum permissions necessary initially and then leverage incremental consent as needed.
@gavinbarron These are the default permissions are openid, profile and User.Read.
The two permissions required for unconsenting require admin consent unlike the current default permissions. With regard to and appreciation of the principle of least privilege access, then the best option would be to let the user consent to them when they need to revoke a permission.
@Onokaev per our conversation today during the sync:
- Can we please ensure that the requirements stated on this thread are followed (text content, UI, etc)
- Consent to permissions modal should be consistent with the permissions tab. Action for me: Consult with the UX team to come up with a good filtering way.
- Tested and the filtering is still not quite correct
@Onokaev the consent/revoke button is the most important functionality on this screen and yet I can't see it properly. In comparaison the description is taking too much horizontal space.
@gavinbarron could you please assist us with this . Thank you :)
@RabebOthmani what did you try to filter? I just tested and it's working on my end I have made the changes to the text content, so that's covered. I have also done a quick fix on the length of the description. Let me know how that looks on your screen :)
@RabebOthmani happy to lend a hand here, although it looks like @Onokaev has added some text wrapping and a fix to ensure that the Consent/Unconsent button is not obscured by the scrollbar. Not sure that there's much value for me to add on this.
How are we filtering the list of permissions cc @adhiambovivian
Hey @RabebOthmani. This filtering requires some work on the DevX API repo
Ticket tracking filtering of permissions: https://github.com/microsoftgraph/microsoft-graph-devx-api/issues/1232