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

Support for user assigned identity for App Service MSI #5137

Closed
noelbundick opened this issue Jan 11, 2019 · 7 comments
Closed

Support for user assigned identity for App Service MSI #5137

noelbundick opened this issue Jan 11, 2019 · 7 comments

Comments

@noelbundick
Copy link

@noelbundick noelbundick commented Jan 11, 2019

Per https://docs.microsoft.com/en-us/azure/app-service/overview-managed-identity, the Azure App Service token URL, as accessed by MSI_ENDPOINT supports an optional parameter named clientid to select which User-Assigned Identity to use.

The current call to that endpoint does not pass through the clientid parameter for App Service MSI, only for VM IMDS endpoint calls. See: https://github.com/Azure/azure-sdk-for-net/blob/psSdkJson6/src/SdkCommon/AppAuthentication/Azure.Services.AppAuthentication/TokenProviders/MsiAccessTokenProvider.cs#L56

This means that effectively, Microsoft.Azure.Services.AppAuthentication 1.2.0-preview cannot be used to retrieve tokens for user-assigned MSI on Azure App Service / Azure Functions.

I think this is a relatively simple change to add the clientid parameter to the app service MSI call

Would you be open to a PR (with tests, of course) to add this feature?

@michaelwstark
Copy link

@michaelwstark michaelwstark commented Jan 25, 2019

@noelbundick
If you are looking for a work around - the following works. Definitely not ideal and could easily break when\if you update the version of the package you are consuming. It works because the parameters being appended to the url are not being properly escaped in the source:

        AzureServiceTokenProvider tokenProvider = new AzureServiceTokenProvider();

        return new KeyVaultClient(
                (authority, resource, scope) =>
                {
                    resource = string.Format(CultureInfo.InvariantCulture, "{0}&clientid={1}", resource, clientId);
                    return tokenProvider.KeyVaultTokenCallback(authority, resource, scope);
                });
@noelbundick
Copy link
Author

@noelbundick noelbundick commented Jan 25, 2019

Thanks @michaelwstark

For me, I don't want to use KeyVault at all - I just want to hit MSI_ENDPOINT, get a token, and then use it against ARM.

I've actually had working code for this for a while now (not-terrible commit w/ changes but no tests here: noelbundick/azure-utilities@debdc8b). It can be used w/ user-assigned MSI on Functions, and then falls back to azure-cli auth for local dev - so you don't have to jump through a lot of hoops just to get an auth token and call Azure.

My intent was more to ask the question "If I make the effort to set up the test suite, write tests, get everything working, write docs, make PRs to Azure docs, etc" is this package even open to accepting a PR?

@michaelwstark
Copy link

@michaelwstark michaelwstark commented Jan 25, 2019

@noelbundick
The workaround should be pretty similar with or without key vault -- internally the callback just calls the same method you have referenced in your original comment.

Unfortunately, I can't answer the question regarding the contribution PR as I don't own or maintain this repo. I would hope the maintainers would welcome the contribution, but they would have to confirm. I'm just another developer that wanted to use user assigned MSI and ran into this issue.

@noelbundick
Copy link
Author

@noelbundick noelbundick commented Jan 25, 2019

For any adventurous comment readers that want to do what I did to get unblocked

@nonik0
Copy link
Contributor

@nonik0 nonik0 commented Mar 11, 2019

FYI this support is added in 1.2.0-preview2.

@varunsh-msft
Copy link
Member

@varunsh-msft varunsh-msft commented Mar 27, 2019

@shahabhijeet , can you please close this? Thanks!

@varunsh-msft
Copy link
Member

@varunsh-msft varunsh-msft commented Apr 6, 2019

Closing as this is done in 1.2.0-preview2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants