Skip to content
This repository has been archived by the owner on Apr 10, 2024. It is now read-only.

Adds user assigned identity for web apps #124

Merged
merged 2 commits into from
Jun 7, 2019

Conversation

noelbundick
Copy link
Contributor

Adds support for user-assigned managed identity on webapps by adding the clientid parameter to the request made to MSI_ENDPOINT per the docs

I've added tests and manually verified outside of this repo that this correctly acquires an access token on a Python Azure Function on App Service Plan

Other relevant info:

  • User-assigned identity on consumption plan doesn't work yet
  • For App Service/Functions, the parameter name is clientid, while for IMDS, it is client_id ☹️ The parameter name in the PR is not a typo
  • At the time of this PR, builds on master are broken - I'll rebase and update once master is green, just want to start the ball rolling on this

"access_token": "AccessToken"
}
httpretty.register_uri(httpretty.GET,
'http://127.0.0.1:41741/MSI/token/?resource=foo&api-version=2017-09-01&clientid=bar',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a discussion with Arturo right now that this will be changed to client_id. Do you have any insights?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I sure don't, sorry - wasn't aware of those discussions

I would actually love to see this be made consistent with VM IMDS, and then we simplify everything to client_id everywhere. If/when this is resolved, please do share :)

I have an open PR on the .NET side that would need the same changes (Azure/azure-sdk-for-net#5181)

Copy link
Member

@lmazuel lmazuel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, just a question about "clientid" against "client_id" since there is a discussion right now, I'd like to merge the final recommendation if possible. Thanks!

@lmazuel
Copy link
Member

lmazuel commented Jun 7, 2019

Hi @noelbundick
Sorry it took so long... Mostly it's because I got issue to test it, and and then it got push out of my todolist...
I was able to succesfully test it today, so merging the PR :)
Thanks for the help!

@lmazuel lmazuel merged commit 051f7ac into Azure:master Jun 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants