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 Azure.Core.TokenCredential (and thus Azure.Identity) in the client #1971

Closed
drdamour opened this issue Oct 29, 2020 · 25 comments
Closed
Labels
feature-request New feature or request

Comments

@drdamour
Copy link

drdamour commented Oct 29, 2020

Is your feature request related to a problem? Please describe.
I'm always frustrated when i have to copy paste secrets into my config/environment

Describe the solution you'd like
I'd like the CosmosClient (and any factory like things for making it) take a Azure.Core.TokenCredential instance that fetches the secrets using the fall thru mech of Azure.Identity (which supports managed identity and az cli for local dev). EG do all the stuff at https://docs.microsoft.com/en-us/azure/active-directory/managed-identities-azure-resources/tutorial-windows-vm-access-cosmos-db but with the azure.core.tokencredential.

I'd also like for the connection string constructor to support a stanza like Authentication=DefaultAzureCredential which means it would use a default constructor instance of DefaultAzureCredential class. this means that for other libraries (like Cosmos Azure Functions Extension) i can (assuming libs are up to date) just udpate the connection string as such an extension doesn't let me override CosmosClient construction directly but the connection string is respected. Really any of the TokenCredentials with default constructors should be used.

Describe alternatives you've considered
write my own factory wrapper for the client...but this is pretty basic functionality.

Additional context
EventHub client has moved towards TokenCredential, seems like others are too.

also if cosmos begins to natively support manage identity (and not just the shared key stuff) it can leverage that instead.

It'll need to try to get read/write and fall back to read only key somehow as well

@ealsur ealsur added the feature-request New feature or request label Oct 29, 2020
@j82w
Copy link
Contributor

j82w commented Oct 30, 2020

There is work in progress for Cosmos DB to support Azure Active Directory. There are plans to expose the TokenCredential interface on the CosmosClient, but the Azure Activity Directory needs to be at least in public preview first.

@drdamour
Copy link
Author

@j82w i hear you...but if the client can support TokenCredential already by getting the keys for us at runtime that's still a big win

@V4A001
Copy link

V4A001 commented Feb 7, 2021

I totally agree something easier with Azure.Identity could be in place. This takes too much time to accomplish. Secondly, the cost of azure cosmos db is too much if you have limited to no data or limited to no throughput :-(.

This is my code based on different samples around. We use it in a AKS cluster with aadpodidentities.

public class DatabaseAccountListKeysResult { [JsonPropertyName("primaryMasterKey")] public string PrimaryMasterKey { get; set; } [JsonPropertyName("primaryReadonlyMasterKey")] public string PrimaryReadonlyMasterKey { get; set; } [JsonPropertyName("secondaryMasterKey")] public string SecondaryMasterKey { get; set; } [JsonPropertyName("secondaryReadonlyMasterKey")] public string SecondaryReadonlyMasterKey { get; set; } }

`public static class ServiceCollectionExtensions
{
public static IServiceCollection AddCosmosDb(this IServiceCollection services, IConfiguration configuration)
{
CosmosDbConfiguration options = configuration.GetSection(CosmosDbConfiguration.SectionName)
.Get();

        services.AddSingleton<ICosmosDbService>(InitializeCosmosClientInstanceAsync(options).GetAwaiter().GetResult());

        return services;
    }      
    /// <summary>
    /// Creates a Cosmos DB database and a container with the specified partition key. 
    /// </summary>
    /// <returns></returns>
    private static async Task<CosmosDbService> InitializeCosmosClientInstanceAsync(ICosmosDbConfiguration cosmosDbConfiguration)
    {
        // AzureServiceTokenProvider will help us to get the Service Managed token.
        var azureServiceTokenProvider = new AzureServiceTokenProvider();

        // Authenticate to the Azure Resource Manager to get the Service Managed token.
        string accessToken = await azureServiceTokenProvider.GetAccessTokenAsync("https://management.azure.com/");

        // Setup the List Keys API to get the Azure Cosmos DB keys.
        
        string endPoint = $"https://management.azure.com/{cosmosDbConfiguration.Scope}/listKeys?api-version=2019-12-12";

        // Setup an HTTP Client and add the access token.
        HttpClient httpClient = new HttpClient();
        httpClient.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", accessToken);

        // Post to the endpoint to get the keys result.
        var result = await httpClient.PostAsync(endPoint, new StringContent(""));

        // Get the result back as a DatabaseAccountListKeysResult.
        DatabaseAccountListKeysResult keys = await result.Content.ReadAsAsync<DatabaseAccountListKeysResult>();

        //CosmosClient client = new CosmosClient(account, key);
        CosmosClient client = new CosmosClient(cosmosDbConfiguration.Account, keys.PrimaryMasterKey);
        CosmosDbService cosmosDbService = new CosmosDbService(client, cosmosDbConfiguration.DatabaseName, cosmosDbConfiguration.ContainerName);
        DatabaseResponse database = await client.CreateDatabaseIfNotExistsAsync(cosmosDbConfiguration.DatabaseName);
        await database.Database.CreateContainerIfNotExistsAsync(cosmosDbConfiguration.ContainerName, $"/{Company.PartitionKey}");

        return cosmosDbService;
    }`

setup with Azure cli:
`COSMOSDB_ID="$(az identity create -g ${RESOURCE_GROUP} -n ${COSMOSDB_IDENTITY_NAME} --query id -otsv)"
export COSMOSDB_ID="$(az identity show -g ${RESOURCE_GROUP} -n ${COSMOSDB_IDENTITY_NAME} --query id -otsv)"
echo $COSMOSDB_ID

export COSMOSDB_IDENTITY_PRINCIPAL_ID=$(az identity show -g ${RESOURCE_GROUP} -n ${COSMOSDB_IDENTITY_NAME} --query principalId -otsv)
echo $COSMOSDB_IDENTITY_PRINCIPAL_ID

#set managed identity to access the identity resource group
scope=$(az cosmosdb show --name $COSMOSDB_ACCOUNTNAME --resource-group ${RESOURCE_GROUP} --query id)
echo $scope
az role assignment create --role "DocumentDB Account Contributor" --assignee ${COSMOSDB_IDENTITY_PRINCIPAL_ID} --scope $scope #/subscriptions/${SUBSCRIPTION_ID}/resourcegroups/${RESOURCE_GROUP}/providers/Microsoft.DocumentDb/databaseAccounts/$COSMOSDB_ACCOUNTNAME
`

keyvault:
`az keyvault secret set --vault-name $KEYVAULT_NAME --name "CosmosDb--Account" --value $COSMOSDB_ACCOUNTNAME
az keyvault secret set --vault-name $KEYVAULT_NAME --name "CosmosDb--DatabaseName" --value $COSMOSDB_DBNAME
az keyvault secret set --vault-name $KEYVAULT_NAME --name "CosmosDb--ContainerName" --value $COSMOSDB_CONTAINERNAME

not in use MSI az keyvault secret set --vault-name $KEYVAULT_NAME --name "CosmosDb--Key" --value "" #not in use

az keyvault secret set --vault-name $KEYVAULT_NAME --name "CosmosDb--Scope" --value $scope`

@drdamour
Copy link
Author

drdamour commented Feb 7, 2021

@V4A001 have you tried the serverless/consumption cosmos? i'm in same boat with low volume and been wondering if that takes me from $1 to pennies

@V4A001
Copy link

V4A001 commented Feb 7, 2021

@drdamour : thank you. I saw the preview in portal indeed. Nice! I will check so I can join the boat instead of swimming :-). I believe for azure cli no options are present yet. Need to check it. Also all the security things would be nice to have azure cli sample.

@j82w
Copy link
Contributor

j82w commented May 25, 2021

Closing this issue because the ask is for Azure.Identity to be included in Azure.Core. The Cosmos DB repo can not make that change. Please open an issue on the Azure.Core repository.

@j82w j82w closed this as completed May 25, 2021
@drdamour
Copy link
Author

@j82w i'm not following, Azure.Idnetity is already in Azure.Core, the request is for the Cosmos client to make use of it.

@j82w
Copy link
Contributor

j82w commented May 26, 2021

@drdamour correct me if I misunderstanding something.

Azure.Identity has a dependency on Azure.Core.
Cosmos DB SDK has a dependency on Azure.Core.
Azure.Core provides a interface for authentication.
Azure Identity implements the authentication interface provided by Azure.Core.

The Azure.Core nuget does not have any dependency on Azure.Identity project. https://www.nuget.org/packages/Azure.Core/
The Azure.Identity does have a dependency on Azure.Core https://www.nuget.org/packages/Azure.Identity/

The github projects show that they are in completely different projects.
https://github.com/Azure/azure-sdk-for-net/tree/master/sdk/identity/Azure.Identity/src
https://github.com/Azure/azure-sdk-for-net/tree/master/sdk/core/Azure.Core/src

@galvesribeiro
Copy link

I think there is a misunderstanding here...

The goal, if I understood correctly, is to allow Service Principals / Managed Identities to authenticate with CosmosDB.

If that is the case, all that CosmosDB SDK needs to use is the abstract Credentials Classe which is implemented both by Service Principal, Token Credentials, Managed Identities and all sorts of identity types that are offered thru Azure Active Directory.

@Matthewsre
Copy link

Matthewsre commented May 26, 2021

@j82w

I started following this issue with the expectation that I would be able to provide a Azure.Core.TokenCredential to the CosmosClient.

I can provide this for

Azure.Security.KeyVault.Secrets.SecretClient:

image

Azure.Messaging.ServiceBus.ServiceBusClient:

image

Azure.Storage.Files.DataLake.DataLakeServiceClient:

image

I would like this to be part of the constructor for CosmosClient as well, and I don't mean write my own custom code to use the TokenCredential to go get a key and then pass that in. I can grant the permissions, but would want to use the CosmosClient with the same ease of use and consistency of the other clients.

I understand there might be challenges with how these different services were built since Cosmos is a bit different with the Primary and Secondary keys, but this is what would make me happy as a consumer. This also seems to fit the initiative with the different Azure.* SDKs that I was learning about when I found out I had to switch to another new SDK for ServiceBus to get this functionality. :)

@galvesribeiro
Copy link

galvesribeiro commented May 26, 2021 via email

@drdamour
Copy link
Author

@j82w exactly what @Matthewsre said, this is about having cosmos take core’s token credential, so i can pass in those from
azure.Identity like the other azure sdks. Please reopen, ill update the description

@drdamour drdamour changed the title support Azure.Identity in the client support Azure.Core.TokenCredential (and thus Azure.Identity) in the client May 26, 2021
@MPapst
Copy link

MPapst commented May 26, 2021

Do you guys mean this ctor for CosmosClient?

@j82w
Copy link
Contributor

j82w commented May 26, 2021

@galvesribeiro, @Matthewsre, @galvesribeiro, @drdamour please check out the latest 3.19.0 which makes the TokenCredential part of the GA SDK as @MPapst pointed out.

The preview SDK has had this exposed for several months, and it was moved to the GA SDK as part of the 3.19.0 release.

public static async Task<CosmosClient> CreateAndInitializeAsync(string accountEndpoint,

@V4A001
Copy link

V4A001 commented May 26, 2021

Is this preview also available as nuget?

@MPapst
Copy link

MPapst commented May 26, 2021

It is available as GA.

https://www.nuget.org/packages/Microsoft.Azure.Cosmos/

@galvesribeiro
Copy link

ahh ok, thanks!

@galvesribeiro
Copy link

Hey folks!

I just would like to let you all know about a caveat that I've recently be notified by some folks on another OSS project I maintain that, even thought now TokenCredential is supported at the SDK, it is not possible to do management operations with it like create Database or Collections.

You may see more details here: https://docs.microsoft.com/en-us/azure/cosmos-db/how-to-setup-rbac#permission-model

The alternative is to have a separated client which uses the AuthKey to perform those operations and use the "data" client with the TokenCredential.

That beats the whole purpose of using TokenCredential and not having to share the AuthKey with the clients.

@MPapst @j82w do you have any ideas if this is something that will be supported? Having 2 clients and sharing the AuthKey among with the app makes no sense to me.

This issue should be re-opened as the feature was not actually fully implemented ☹️

@FabianMeiswinkel
Copy link
Member

Hi @galvesribeiro, just wanted to elaborate on this.
The SDK - as in the .Net SDK 3.19.0 allows using AAD/RBAC for data plane operations only. There is a separate SDK (see ) for executing control plane operations - like creating account, creating/modifying databases and containers etc. - and that SDK also allows you to use AAD/RBAC. The challenge is that when using MasterKey based authentication some operations like creating containers were initially allowed via the SDK (data plane operation) as well as via the control plane SDK (via Azure Resource Manager). There has been an option to disallow these operations from the data plane for customers, who wanted to ensure proper auditing (by enforcing AAD authentication/RBAC) for these operations. The Role based permission model for control plane and data plane are different - and as a result we had to constraint the usage form within the Core SDK to all data plane operations. Going through the control plane (and the management SDK) for management operations like creating containers has been the recommendation for a while already - and some features like moving back-and-forth between AutoScale and Manual Throughput are only available in the Management SDK.
I hope that explanation makes sense and makes it understandable why the limitation above exists.

Couple of pointers to further documentation below:

@galvesribeiro
Copy link

@FabianMeiswinkel thanks for the quick reply.

I understand and actually agree 100% with you that those management operations should be separated from the data plane. In fact, in production, we make sure the application only use credentials with the data read/write/query permissions.

However, the Management NuGet is in preview for a while so we can't use it in production NuGets. I think the removal of management capabilities on the data plane SDK, regardless of using AuthToken or TokenCredential should only happen when we have the Management NuGet published as a stable package and when that happen, I would even go further and make sure all those CreateXXXIfNotExist methods and the indexing configuration APIs get completely removed from the data plane library to avoid confusion.

Is there a plan to make those management packages stable?

@V4A001
Copy link

V4A001 commented Jun 27, 2021

I wanted to update my code as I am still missing the primary key on multiple pods.

Can somebody show me "how to dance" and have this work with managed identities and this token? I liked that from this paragraph:

"So, it all bail down to whether or not Cosmos DB support TokenCredentials (and managed identities) at the service level. If it doesn’t, the SDK should accept the TokenCredentials as you mentioned and internally do all the dance to acquire CosmosDB keys and use it."

It is similar to accessing the keyvault with a managed identity? I am looking indeed for the last.

         `   // Configure Azure Key Vault Connection
                 var keyVaultEndPoint = $"https://{keyVaultName}.vault.azure.net/";

                 //use managed identity.
                 var secretClient = new SecretClient(new Uri(keyVaultEndPoint), new DefaultAzureCredential(new DefaultAzureCredentialOptions { ExcludeVisualStudioCodeCredential = true }));

                 configBuilder.AddAzureKeyVault(secretClient, new KeyVaultSecretManager());`

Maybe update the documentation here:
https://docs.microsoft.com/en-us/azure/cosmos-db/managed-identity-based-authentication

@V4A001
Copy link

V4A001 commented Jun 28, 2021

I have coded next:

//use managed identity. var credential = new DefaultAzureCredential(new DefaultAzureCredentialOptions { ExcludeVisualStudioCodeCredential = true }); CosmosClient client = new( $"https://{cosmosDbConfiguration.Account}.documents.azure.com:443/", credential, //keys.PrimaryMasterKey, new CosmosClientOptions() { SerializerOptions = new CosmosSerializationOptions() { PropertyNamingPolicy = CosmosPropertyNamingPolicy.CamelCase } }); CosmosDbService cosmosDbService = new(client, cosmosDbConfiguration.DatabaseName, cosmosDbConfiguration.ContainerName); DatabaseResponse database = await client.CreateDatabaseIfNotExistsAsync(cosmosDbConfiguration.DatabaseName); await database.Database.CreateContainerIfNotExistsAsync(cosmosDbConfiguration.ContainerName, $"/{Company.PartitionKeyPath}");

I get next error on my local dev box account. I will investigate further.
Microsoft.Azure.Cosmos.CosmosException: 'Response status code does not indicate success:
Forbidden (403); Substatus: 5301; ActivityId: 566e318c-b8fc-4864-9238-527d383bf550; Reason:
(Request blocked by Auth erp2erp-cosmosdb-001 : Request is blocked because principal [xx] does not have required RBAC permissions to perform action [Microsoft.DocumentDB/databaseAccounts/readMetadata] on resource [/]. Learn more: https://aka.ms/cosmos-native-rbac.
ActivityId: 566e318c-b8fc-4864-9238-527d383bf550, Microsoft.Azure.Documents.Common/2.14.0, Windows/10.0.19043 cosmos-netstandard-sdk/3.19.3);'

Assigning DocumentDB Account Contributor to my account did not help.

@j82w
Copy link
Contributor

j82w commented Jun 28, 2021

@V4A001 as Fabian previously explained the CosmosClient can NOT do management operations(create/delete/update) on databases and/or containers. Is there something we can change about the important notice at the top of the https://aka.ms/cosmos-native-rbac to be more clear that this scenario is not supported?

@galvesribeiro
Copy link

@j82w that is clear. But my point is, we can't have something removed like this without having something else stable to cover.

Do you have a plan to make the management libraries stable? It is being 9 months that this package is there but never made its way to stable and we can't use it.

Also, if the client isn't supposed to do management operations, shouldn't you remove methods like CreateXXXIfNotExist() from it to avoid confusion?

@hansmbakker
Copy link

Agree, it's a super bad developer experience to have the client.CreateDatabaseIfNotExistsAsync method available but getting runtime errors when using a TokenCredential.

Yes, this is called out in the documentation.
No, it is not intuitive at all.

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

No branches or pull requests

9 participants