-
Notifications
You must be signed in to change notification settings - Fork 110
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
Bugfixes + extra validation + tenant support on api calls #41
Conversation
Added extra validation based on the client_id and the appid from the JWT Token claim. If you use the client_credentials grant type the "aud" is "https://graph.windows.net/" instead of the client_id. The client_id in this case is located in the "appid" claim.
\
for RuntimeExceptioncommon = default on azure graph api
Added tenant support in api calls
What does the tenant support on api calls do exactly? Can you please provide a sample where this is used? The reason why I am asking is because there are different versions of odata spec, in which (MS Graph) returns full URL, where (Azure Graph) returns only the address part without tenant. So I think this could technically break things. |
If you have an OAuth2 application that does api calls for the user instead of the user itself. For example if you have an OAuth2 application setup with the client_credentials grant you request oauth tokens like this: `POST /2337dcc6-3a21-4d95..../oauth2/token HTTP/1.1 ------WebKitFormBoundary7MA4YWxkTrZu0gW https://graph.windows.net/2337dcc6-3a21-4d95..../.default client_credentials And you get back:
If then you want to do an api call to get for example all the users you need to include the tenant in the uri:
If you don't include the tenant information you'll get the following response: https://graph.windows.net/users?api-version=1.6
I'll change the commit to remove the 'common' and only add the tenant if it's set as an option if that's ok for you. If you need more info feel free to ask or decline PR. |
I've updated the file to make sure it stays as it is by default and only add the tenant if it's set as an option on the provider. |
Let's do it a bit different then: Here's a detection whether the nextLink is URL - https://github.com/NoScopie/oauth2-azure/blob/b031aac960bc334debaac8141b5fe03188cb889e/src/Provider/Azure.php#L181 I would defintiely start there. So if the nextLink is not URL, check if it is meant for Azure Graph, then do the magic with tenant. Also, I am not sure if |
Hi, sorry for the late reply. Atm I don't have time to look into this further due to a large project I'm working on. I'll try to get back to this asap. |
I am not going to merge this PR, instead I reworked the changes in 3163106 If this is alright with you @NoScopie, I can release this as a new version. |
Closes #34 + Added extra validation on client id if you use client_credentials grant