Azure / azure-cosmos-dotnet-v3

.NET SDK for Azure Cosmos DB for the core SQL API
MIT License
737 stars 493 forks source link

Management operations using TokenCredential failing #2394

Open SumiranAgg opened 3 years ago

SumiranAgg commented 3 years ago

We are continuously addressing and improving the SDK, if possible, make sure the problem persist in the latest SDK version.

Describe the bug I gave an AAD identity DatabaseAccountContributor role from azure portal and /readMetadata and /container/* permissions as per RBAC control for dataplane operations (Referred this doc https://docs.microsoft.com/en-us/azure/cosmos-db/how-to-setup-rbac). When trying to create a container using cosmos client initialized with this identity I see below error:

"The given request [POST /dbs/MetadataDatabase/colls] cannot be authorized by AAD token in data plane."

Am I doing something wrong or we do not have support to perform management operations using client created using AAD credentials?

To Reproduce Covered above

Expected behavior All operations should be successful.

Actual behavior Create container call failed.

Environment summary SDK Version: 3.18.0-preview OS Version (e.g. Windows, Linux, MacOSX)

Additional context Add any other context about the problem here (for example, complete stack traces or logs).

j82w commented 3 years ago

Currently the .NET and Java SDK do not support control plane operation via Azure Active Directory. Those management operations can be done via the via ARM Templates, PowerShell, Azure CLI, REST, or Azure Management Library.

SumiranAgg commented 3 years ago

Is the feature development on roadmap? Is yes do we have an ETA on when can we expect the support.

j82w commented 3 years ago

The feature is currently not being prioritized, because the control plane operations can be done via Azure Management Library which has a c# nuget.

qweeah commented 3 years ago

Also met this issue when moving from using Cosmos DB string to Azure Managed Identity in our service.

@j82w I can see that both Azure.ResourceManager.CosmosDB and Microsoft.Azure.Management.CosmosDB are available on Nuget, but the former is not updated for over 9 months, is it expected to be replaced by the latter?

The feature is currently not being prioritized, because the control plane operations can be done via Azure Management Library which has a c# nuget.

rjygraham commented 3 years ago

To understand why this needs to be prioritized, someone on the Cosmos DB team should do the following:

  1. Follow the Quickstart: Build a .NET console app to manage Azure Cosmos DB SQL API resources
  2. Run the quickstart and observe successful execution
  3. Modify the quickstart to use DefaultAzureCredential/managed identity for authentication & grant RBAC permissions on the Cosmos DB account
  4. Run the quickstart and observe all the failures
  5. Go through the pain of modifying the quickstart to work 100% with managed identities leveraging multiple SDKs and authentication mechanisms.
bedding commented 3 years ago

I ran into the same issue lately and I am surprised this is not prioritized. The Azure.Cosmos SDK provides implementation for managing database and container while the service is blocking the operation from SDK, which seems to be inconsistent.

tkrille commented 2 years ago

5. Go through the pain of modifying the quickstart to work 100% with managed identities leveraging multiple SDKs and authentication mechanisms.

How do you even do this? I tried to get it work and after several hours came up with this:

val subscriptionId = ResourceManager
    .authenticate(credential, AzureProfile(AzureEnvironment.AZURE))
    .withDefaultSubscription()
    .subscriptionId()
Configuration.getGlobalConfiguration().put(PROPERTY_AZURE_SUBSCRIPTION_ID, subscriptionId)
CosmosManager
    .authenticate(credential, AzureProfile(AzureEnvironment.AZURE))
    .serviceClient()
    .sqlResources
    .createUpdateSqlContainer(
        resourceGroup,
        account,
        database,
        containerName,
        SqlContainerCreateUpdateParameters()
            .withOptions(CreateUpdateOptions().withThroughput(throughput))
            .withResource(
                SqlContainerResource()
                    .withId(containerName)
                    .withPartitionKey(ContainerPartitionKey().withPaths(listOf(partitionKeyPath)))
                    .withIndexingPolicy(
                        IndexingPolicy()
                            .withIncludedPaths(listOf(IncludedPath().withPath(includePath)))
                            .withExcludedPaths(listOf(ExcludedPath().withPath(excludePath)))
                    )
            )
    )

Is this even a good solution? This is in Kotlin with Java SDK, so might not really fit in here, but as you can see, it's a lot of code just to cover a basic use case. It would be really nice, if the Cosmos DB SDK could do management operations with token credentials.

kxl307 commented 2 years ago

Is there a plan to fix this? At least we want the collection resource to be supported. I thought rbac supports all operations in Azure is the future.

blinard commented 2 years ago

I understand that there's a difference between data-plane and control-plane RBAC but having a core part of this SDK's functionality broken when using TokenCredential for authentication is a really poor developer experience.

To add insult to injury, none of our documentation calls this out and our error messaging for this issue is fairly obtuse.

Could we, at the very least, take a priority story to update our documentation to reflect this constraint when using TokenCredentials? Also, perhaps better yet, could we throw a custom/clear exception (or output an obvious debugger/console message if an exception is deemed too risky) to callout that resource creation (databases, containers, etc.) are not supported when using TokenCredentials? I would think that we could identify the type of credentials being used from the implementation logic of the impacted methods to facilitate such messaging.

TokenCredential support was added to this SDK back in May. It's now 6 months later. I can understand not prioritizing the issue initially but it seems like we should do something about it now. Even if it's just adding appropriate messaging and updating our docs to reflect the constraint. Obviously it'd be better to actually fix the issue b/c expecting devs to integrate multiple nuget packages and roll their own logic to ensure containers are created is a pretty poor experience too (especially when that concept is already supported within this sdk).

ViSilver commented 1 year ago

Are there any updates on the matter?

bhanuprakash-1 commented 1 year ago

I understand that there's a difference between data-plane and control-plane RBAC but having a core part of this SDK's functionality broken when using TokenCredential for authentication is a really poor developer experience.

To add insult to injury, none of our documentation calls this out and our error messaging for this issue is fairly obtuse.

Could we, at the very least, take a priority story to update our documentation to reflect this constraint when using TokenCredentials? Also, perhaps better yet, could we throw a custom/clear exception (or output an obvious debugger/console message if an exception is deemed too risky) to callout that resource creation (databases, containers, etc.) are not supported when using TokenCredentials? I would think that we could identify the type of credentials being used from the implementation logic of the impacted methods to facilitate such messaging.

TokenCredential support was added to this SDK back in May. It's now 6 months later. I can understand not prioritizing the issue initially but it seems like we should do something about it now. Even if it's just adding appropriate messaging and updating our docs to reflect the constraint. Obviously it'd be better to actually fix the issue b/c expecting devs to integrate multiple nuget packages and roll their own logic to ensure containers are created is a pretty poor experience too (especially when that concept is already supported within this sdk).

Totally Accept. Its very bad developer experience. Had spent a lot of time only to find that control plane operations are not supported when used TokenCredential(but wierdly is supported with other auth mechanisms). Atleast the error thrown should be pointing it out.

Any update on this or can someone point me to a Learn tutorial where we can do cosmos DB management operations using .net SDK using TokenCredential...it would be really helpful. I expect something like has a very good documentation and a quick start tutorial when Azure itself recomends to use ManagedIdentitles based auth.

johncrim commented 1 year ago

Currently the .NET and Java SDK do not support control plane operation via Azure Active Directory. Those management operations can be done via the via ARM Templates, PowerShell, Azure CLI, REST, or Azure Management Library.

@j82w - thank you, this is helpful.

However, it really should be highlighted in docs/examples/API documentation instead of left for developers to figure out/find this issue in github. Especially given that using ManagedIdentity is a best practice, and all these APIs (eg create container) are in the client library and work when you're using an emulator, but stop working when running in Azure.

hansmbakker commented 1 year ago

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.