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

TokenCredential support for ShareClient #22753

Closed
Kruti-Joshi opened this issue Jul 20, 2021 · 10 comments
Closed

TokenCredential support for ShareClient #22753

Kruti-Joshi opened this issue Jul 20, 2021 · 10 comments
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Service Attention Workflow: This issue is responsible by Azure service team. Storage Storage Service (Queues, Blobs, Files)

Comments

@Kruti-Joshi
Copy link

Query/Question
I'm using Azure.Identity package to use Managed Service Identity for authorization between the different Azure components in our project. One requirement is to connect to a File Share from an Azure Function. I notice that there is TokenCredential support in BlobClient, QueueClient and the DataLakeClients, but not in ShareClient. Is there any plan of adding this support in the near future? To use MSI for File Share, should I use the AppAuthentication library instead?

Environment:

  • Name and version of the Library package used: Azure.Storage.Files.Share 12.7.0; Azure.Identity 1.4.0
  • Hosting platform or OS and .NET runtime version:

image

  • IDE and version : Visual Studio 2019 Version 16.10.2
@ghost ghost added needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. customer-reported Issues that are reported by GitHub users external to the Azure organization. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Jul 20, 2021
@jsquire jsquire added Client This issue points to a problem in the data-plane of the library. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team Service Attention Workflow: This issue is responsible by Azure service team. Storage Storage Service (Queues, Blobs, Files) labels Jul 20, 2021
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Jul 20, 2021
@ghost
Copy link

ghost commented Jul 20, 2021

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @xgithubtriage.

Issue Details

Query/Question
I'm using Azure.Identity package to use Managed Service Identity for authorization between the different Azure components in our project. One requirement is to connect to a File Share from an Azure Function. I notice that there is TokenCredential support in BlobClient, QueueClient and the DataLakeClients, but not in ShareClient. Is there any plan of adding this support in the near future? To use MSI for File Share, should I use the AppAuthentication library instead?

Environment:

  • Name and version of the Library package used: Azure.Storage.Files.Share 12.7.0; Azure.Identity 1.4.0
  • Hosting platform or OS and .NET runtime version:

image

  • IDE and version : Visual Studio 2019 Version 16.10.2
Author: Kruti-Joshi
Assignees: -
Labels:

Client, Service Attention, Storage, customer-reported, needs-team-attention, needs-triage, question

Milestone: -

@kasobol-msft
Copy link
Contributor

@Kruti-Joshi None of File Share Data Plane REST API (which is what that SDK calls behind the scenes) support Azure AD (and MSI). Therefore lack of TokenCredential in ShareClient.

@Kruti-Joshi
Copy link
Author

Got it. Thanks @kasobol-msft. Closing this.

May I ask, what is teh reason behind File Share not supporting AAD but the other offerings do?

@kasobol-msft
Copy link
Contributor

@Kruti-Joshi At this moment File Shares offer AAD support if used via SMB protocol, see here. It's just not an option in the REST API. The mainstream scenario for shares is that users (who are granted limited permissions) would mount them to a VM rather than interact over REST. Therefore REST API is available for account owner only (one that has access to account keys) to perform administrative actions (automated or not).

@Matthewsre
Copy link

@kasobol-msft,

I thought TokenCredential support was going to be standard in all of the Azure.* SDKs. I believe it was ServiceBus that had the same type of challenge where keys are used, but they were still able to handle leveraging TokenCredential to get the keys and handle getting the required credentials under the hood. I just want to use my TokenCredential with the proper role assignments that would enable getting the proper credentials for this to work. Is this something that could be added to align with the rest of the Azure.* SDKs?

It looks like this was an option in the V11 SDK using StorageCredentials(TokenCredential).

https://docs.microsoft.com/en-us/dotnet/api/microsoft.azure.storage.auth.storagecredentials?view=azure-dotnet

Could that approach also be used in V12 SDK to bridge the gap?

@kasobol-msft
Copy link
Contributor

Such magic didn't exist in prior Storage SDK versions. I.e. SDK would never attempt to exchange one credential for the other one. The user is responsible for providing one.
Implicitly exchanging token credential for shared key might bring unwanted elevation of privilege. I.e. having shared key in hand is an equivalent of superuser access and if leaked it poses significant threat. Having said that there are APIs that require it and won't accept token credential which ultimately requires caller to have shared key in hand, but this should be an explicit action of the caller to do so.
/cc @schaabs

@Matthewsre
Copy link

Such magic didn't exist in prior Storage SDK versions. I.e. SDK would never attempt to exchange one credential for the other one. The user is responsible for providing one.
Implicitly exchanging token credential for shared key might bring unwanted elevation of privilege. I.e. having shared key in hand is an equivalent of superuser access and if leaked it poses significant threat. Having said that there are APIs that require it and won't accept token credential which ultimately requires caller to have shared key in hand, but this should be an explicit action of the caller to do so.
/cc @schaabs

@kasobol-msft,

From the link I referenced, it looks like you can use the StorageCredential(TokenCredential) and get a CloudFileShare.

Here is the StorageCredential(TokenCredential) constructor:

image

Copied the relevant code samples from the link and only changed the StorageCredential to use the new Constructor:

// Changing storageCredential to use constructor with TokenCredential (only change in this code)
// StorageCredentials storageCredentials = new StorageCredentials(myAccountName, myAccountKey);
StorageCredentials storageCredentials = new StorageCredentials(tokenCredential);
CloudStorageAccount cloudStorageAccount = new CloudStorageAccount(storageCredentials, useHttps: true);

...

// Create a CloudFileClient object from the storage account.
// This object is the root object for all operations on the 
// file service for this particular account.
CloudFileClient fileClient = cloudStorageAccount.CreateCloudFileClient();

...

// Get a reference to a CloudFileShare object in this account. 
// This object can be used to create the share on the service, 
// delete the share, list files and directories, etc. This operation 
// does not make a call to the Azure Storage service. It neither 
// creates the share on the service, nor validates its existence.
CloudFileShare share = fileClient.GetShareReference("share1");

Would this not work in the V11 SDK when the alternative constructor was used?

@kasobol-msft
Copy link
Contributor

The code might compile but will produce an error in runtime. The credentials model in prior versions of SDK was not coherent with underlying service capability.

CloudFileClient ctors for reference:
https://github.com/Azure/azure-storage-net/blob/cbe605f9faa01bfc3003d75fc5a16b2eaccfe102/Lib/Common/File/CloudFileClient.Common.cs#L71

@Matthewsre
Copy link

@kasobol-msft , thanks for clarifying and providing quick responses! I'm glad the new solution didn't carry over that issue. :)

To enable this to work in V12 would your team consider taking inspiration for how this was solved in the cosmos SDK?
Azure/azure-cosmos-dotnet-v3#1971 (comment)

In my case I would expect to grant "Storage Account Contributor" role to my MSI (or other TokenCredential depending on my environment) since this role has access to the keys:
image

I know I could grant the permissions and lookup the keys myself, but it would be nice if the SDK could take care of that for me automatically like it does for Cosmos. This approach would also promote using TokenCredential and keep a consistent experience across all of the Azure.* SDKs.

@kasobol-msft
Copy link
Contributor

As far as this comment goes Azure/azure-cosmos-dotnet-v3#1971 (comment) cosmos didn't and is not going to implement anything like this.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that Service Attention Workflow: This issue is responsible by Azure service team. Storage Storage Service (Queues, Blobs, Files)
Projects
None yet
Development

No branches or pull requests

4 participants