-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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 managed identities and service principals #1372
Conversation
@bgavrilMS would you or someone in your team be able to have a brief look over the Service Principal and Managed Identity parts in MicrosoftAuthentication.cs? Thank you! |
#endregion | ||
|
||
#region Helpers | ||
|
||
private async Task RegisterTokenCacheAsync(IPublicClientApplication app, ITrace2 trace2) | ||
private async Task RegisterTokenCacheAsync(IClientApplicationBase app, ITrace2 trace2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For client_credentials, the tokens are placed in app.AppTokenCache
. We recommend not mixing AppTokenCache and UserTokenCache, i.e. just use another file for app token cache.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good spot! I had overlooked the difference between AppTokenCache
and UserTokenCache
.
I don't know of any app token cache that we could share (like we do with other Microsoft developer tools for the user cache). We can just store in our ~/.gcm
data directory for now.
|
||
public static class X509Utils | ||
{ | ||
public static X509Certificate2 GetCertificateByThumbprint(string thumbprint) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's a slight more battle tested way to load certs. I think this is fine for loading by thumbprint, but if loading by name you should exclude expired certs and load the most fresh.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a compelling argument for allowing people to specify something other than a thumbprint (like the cert name, or file path)? Are there common situations where the thumbprint would not be known?
I guess at certificate renewal a new thumbprint would be created, but the certificate would have the same name?
Right now we only have credential.azreposServicePrincipalCertificateThumbprint
/ GCM_AZREPOS_SP_CERT_THUMBPRINT
to specify a thumbprint.
We could add other options like ..CertificatePath
/ .._CERT_PATH
and ..CertificateName
/ .._CERT_NAME
in the future (unless you think it's useful from the get go!) :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure that you serialize cca.AppTokenCache
for client_credentials.
Thanks for spotting this! I've pushed an update that should now correctly serialise the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice! Just a few nitpicky comments about documentation/trace statements.
Rename the lone GetToken method to clarify that this is for user principals (regular user identities). This is in preparation for adding extra principal types including service principals, and managed identities. Also add some XML doc comments to the method.
Refactor the token cache helper methods to allow us to re-use the existing cache registration logic with a different ITokenCache and StorageCreationProperties. This will be useful when we later introduce a confidential client application (for service principals) that needs a different cache location, and uses the AppTokenCache, rather than the User one.
Add support for acquiring a token for a service principal. Either a client secret or certificate can be used to authenticate (the latter being preferred).
Add support for obtaining an access token using either the system-assigned and a user-assigned managed identity.
Add app token cache support for confidential client applications (service principals). This is a different cache than the one for user tokens that is used by public client applications (for normal users). We do not know of any other app token cache that we can share with currently, so we just use our own in the GCM data directory.
Allow a service principal or managed identity to be used to authenticate against Azure Repos. Required information for service principals is specified in Git config or environment variables, as is the ID for a managed identity.
Add tests of the `GetCredentialAsync` method on the `AzureReposHostProvider` using managed identity and service principal.
**Changes:** - Add support for managed identity and service principals in Azure Repos (#1372) - Support universal Gitea OAuth app configuration (#1442) - Set default generic OAuth redirect URI value (#1444) - Drop WPF helpers on Windows (#1417) - Add software rendering override for Windows (#1445, #1453) - Recognise GitLab hosts via WWW-Authenticate header (#1428) - Recognise Bitbucket hosts via WWW-Authenticate header (#1441) - Support GitHub Gist remote URLs (#1402) - Update to Avalonia 11.x (#1383) - Documentation updates (#1416) - Drop unnecessary .NET Framework-specific code (#1447) - Updates to release process (#1386, #1381) - Update code signing certificates (#1431)
**Changes:** _Since 2.4.0:_ - Fix macOS ARM64 tarball contents (#1458) _Since 2.3.x:_ - Add support for managed identity and service principals in Azure Repos (#1372) - Support universal Gitea OAuth app configuration (#1442) - Set default generic OAuth redirect URI value (#1444) - Drop WPF helpers on Windows (#1417) - Add software rendering override for Windows (#1445, #1453) - Recognise GitLab hosts via WWW-Authenticate header (#1428) - Recognise Bitbucket hosts via WWW-Authenticate header (#1441) - Support GitHub Gist remote URLs (#1402) - Update to Avalonia 11.x (#1383) - Documentation updates (#1416) - Drop unnecessary .NET Framework-specific code (#1447) - Updates to release process (#1386, #1381) - Update code signing certificates (#1431)
Add support for both managed identities and service principals for Azure Repos authentication.
Service principals are the "service account" equivalent to normal "users" (user principals). Rather than a password+MFA, service principals (SP) authenticate either with a 'secret' or a certificate - we support both.
Managed identities (MI) are an evolution of service principals whereby the secret/certificate are hidden and managed by Azure. There are two types of managed identities: system-assigned and user-assigned. System-assigned are tied to a particular VM or resource, whereas user-assigned can be shared between VMs/resources.
Azure DevOps recently learned to support SPs and MIs as identities that can be members of orgs and projects. Note that this only applies to AAD-connected Azure DevOps orgs.
This functionality is most compelling for automated scenarios, like CI machines, that need access to Azure Repos.
Fixes #1313