Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Microsoft Graph docs on required permissions #161

Open
tarkatronic opened this issue Aug 3, 2020 · 6 comments
Open

Improve Microsoft Graph docs on required permissions #161

tarkatronic opened this issue Aug 3, 2020 · 6 comments
Assignees
Labels

Comments

@tarkatronic
Copy link
Contributor

Describe the bug
I've managed to get this so that I can log in via AAD. Now, when I attempt to link my GitHub account, the portal is returning a 500 error on the /link url due to Invalid status code: 403 from the backend. I've added a bit of extra debug in and I can see this is coming from the Microsoft Graph API:

{
  error: {
    code: 'Authorization_RequestDenied',
    message: 'Insufficient privileges to complete the operation.',
    innerError: {
      date: '2020-08-03T20:30:06',
      'request-id': '994aeeab-f00a-484b-b591-0d8ef14d21cf'
    }
  }
}

The problem is, I can't suss out what the necessary permissions are. So far, I have granted delegated permissions for:

  • email
  • Mail.Read
  • openid
  • profile
  • User.Read

From what I can tell, this should be more than enough, given that the request is asking for:

?$select=id,mailNickname,userType,displayName,givenName,mail,userPrincipalName

What am I missing from my permissions to get this working properly? Will this even work for a non-MS company?

@tarkatronic
Copy link
Contributor Author

tarkatronic commented Aug 4, 2020

I believe I have managed to find the answer here, thanks to the Microsoft Graph Explorer. When executing a query against /v1.0/users/<id>/, it shows 15 permissions in use. In actuality, once I de-duped those permissions, it narrowed down to 8.

  • User.Read
  • User.Read.All
  • User.ReadBasic.All
  • User.ReadWrite
  • User.ReadWrite.All
  • Directory.AccessAsUser.All
  • Directory.Read.All
  • Directory.ReadWrite.All

This does not changed based on what fields you are querying for.
So my question now is... do I really need to add all of these permissions?? This seems quite excessive just to grab some information about my own profile. Three of these are for global write access, and 5 of them require admin consent! Is there no simpler way to retrieve this? This level of permission makes this start to look like a security risk.

As a note, accessing /v1.0/me/ requires the same de-duped set of permissions.

@jeffwilcox
Copy link
Contributor

There's def. no reason for any kind of write permission!

@tarkatronic
Copy link
Contributor Author

I agree! Unfortunately I'm still getting the same error with my permissions set up like this:
Screen Shot 2020-08-04 at 5 26 51 PM

I can ask our admins to grant those other permissions, but I really would rather not have to...

@jeffwilcox
Copy link
Contributor

I hear you... permissions can take ages to get approved here, too, if at all...

Do you think you need the graph features? Things it would give you:

  • ability to block Guest users in the tenant (probably important)
  • lookup managers, to notify them if you like that, for new repos etc.

Otherwise, we could probably feature flag it out, or find other options... for example, you may have enough info in the AAD passport response to determine if you think they'll work in your directory.

@tarkatronic
Copy link
Contributor Author

I think this may require a combination of solutions. I can't say for sure whether I will need the graph features, but those two uses you listed sure do sound nice to have.

Regardless, I think a feature flag would be good here. That way, the application will at least work by default. And aside from that, it's probably good to add some documentation on these permission requirements, to make this setup easier for any other users.

Happy to help out here however I can. 😄

@ElenaForester
Copy link

I had a similar issue and it was resolved when added Directory.Read.All with Application type
image

@github-actions github-actions bot added the Stale label Oct 11, 2022
@jeffwilcox jeffwilcox self-assigned this Oct 11, 2022
@jeffwilcox jeffwilcox added keep-me and removed Stale labels Oct 11, 2022
@microsoft microsoft deleted a comment from github-actions bot Jan 7, 2024
@jeffwilcox jeffwilcox changed the title Necessary permissions for the Microsoft Graph provider are unclear Improve Microsoft Graph docs on required permissions Jan 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants