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

[Feature Request] MicrosoftIdentityOptions and DefaultCertificateLoader should support user assigned managed identity #1007

Closed
jmprieur opened this issue Feb 23, 2021 · 6 comments · Fixed by #1008
Labels
enhancement New feature or request fixed
Milestone

Comments

@jmprieur
Copy link
Collaborator

Is your feature request related to a problem? Please describe.
Currently, DefaultCertificateLoader only supports system assigned managed identities, whereas customers and partners also need it to support user assigned managed identity

Describe the solution you'd like
Have a new property named UserAssignedManagedIdentityClientId in MicrosoftIdentityOptions so that developers can provide the user assigned managed identity client ID.

public class MicrosoftIdentityOptions
{
 // Previous properties

 /// <summary>
 /// Used, when deployed to Azure, to specify explicitly a user assigned managed identity.
 /// See https://docs.microsoft.com/azure/active-directory/managed-identities-azure-resources/how-to-manage-ua-identity-portal.
 /// </summary>
 string? UserAssignedManagedIdentityClientId {get;set;}
}

For customers using ASP.NET (not core), and therefore directly the DefaultCertificateLoader, also expose it as a static public member of DefaultCertificateLoader

Additional context

@jmprieur jmprieur added the enhancement New feature or request label Feb 23, 2021
@jmprieur jmprieur added this to the 1.8.0 milestone Feb 23, 2021
jmprieur added a commit that referenced this issue Feb 23, 2021
@jmprieur jmprieur modified the milestones: 1.8.0, 1.7.0 Feb 23, 2021
@admsteck
Copy link

admsteck commented Feb 23, 2021

The LoadFromKeyVault method of DefaultCertificateLoader needs both certificate and secret permissions in key vault. In our use case it would be beneficial if different client ids could be provided for the certificate client and secret client used within the LoadFromKeyVault method.

@jmprieur
Copy link
Collaborator Author

@admsteck : do you want to provide a PR that we could examine?

@jmprieur
Copy link
Collaborator Author

PR is #1008

@jmprieur jmprieur linked a pull request Feb 27, 2021 that will close this issue
@jmprieur jmprieur modified the milestones: 1.7.0, 1.8.0 Feb 27, 2021
@jmprieur jmprieur removed the blocked label Feb 27, 2021
@jmprieur
Copy link
Collaborator Author

Wei replied this would be nice to have it to allow customers to pass user assigned MSI clientId from the code instead of changing App service settings,

@admsteck
Copy link

admsteck commented Mar 1, 2021

The LoadFromKeyVault method of DefaultCertificateLoader needs both certificate and secret permissions in key vault. In our use case it would be beneficial if different client ids could be provided for the certificate client and secret client used within the LoadFromKeyVault method.

I retract this comment. It appears our requirements have changed and the original scope of this issue/PR will be sufficient for our needs.

jmprieur added a commit that referenced this issue Mar 4, 2021
proposed fix for #1007
* Adding support for user assigned managed identity
* Update src/Microsoft.Identity.Web/CertificateManagement/DefaultCertificateLoader.cs
Co-authored-by: jennyf19 <jeferrie@microsoft.com>
@jmprieur jmprieur added the fixed label Mar 5, 2021
@jmprieur jmprieur reopened this Mar 5, 2021
@jennyf19
Copy link
Collaborator

Included in 1.8.0 release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants