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

Blob SaS URL generation using existing connection string "UseDevelopmentStorage=true" #12414

Closed
oatsoda opened this issue Jun 2, 2020 · 12 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. feature-request This issue requires a new behavior in the product in order be resolved. 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)
Milestone

Comments

@oatsoda
Copy link

oatsoda commented Jun 2, 2020

Query/Question
Is there any way to create a SaS URL for a Blob using the existing Connection String mechanism?

Environment:

  • Azure.Storage.Blobs 12.4.1

Details
I am currently wiring all my storage stuff up using a single Connection String passed in to the BlobServiceClient. This is very convenient as I can use the "UseDevelopmentStorage=true" in my development appSettings.json without needing to specify all the details. My ARM deployment can then simply grab the connection string at deployment time for each environment.

However, the new way of generating SaS URLs for temporary Blob access requires me to create a new instance of StorageSharedKeyCredential and there doesn't appear to be an automatic way of getting this from an existing instance of BlobServiceClient.

This leaves me with the choices of:

  • Parse the connection string myself (the StorageConnectionString class is marked internal) including detecting "UseDevelopmentStorage=true" manually and splitting string etc.
  • Change everything to use Account Name and Account Key (which means I have to put the development account name and key in explicitly).

Would it not be simpler to have a way of creating a SaS token using a BlobServiceClient - either directly or indirectly - which already has the credentials loaded?

Or at least expose the StorageConnectionString class so I can parse the connection string again. Otherwise I will have to add all this logic again myself.

@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 Jun 2, 2020
@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 Jun 2, 2020
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Jun 2, 2020
@ghost
Copy link

ghost commented Jun 2, 2020

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

@jaschrep-msft
Copy link
Member

Hi @oatsoda. We are currently exploring ways of building a SAS based on an existing client. However, pulling the shared key out of the client is not part of the current design, as there is no guarantee a client has a shared key loaded in. Your feedback on this scenario will be looped into the discussion.

I notice you describe one of your current options as Change everything to use Account Name and Account Key (which means I have to put the development account name and key in explicitly). What exactly is your worry with this? It sounds as if you're worried to store such sensitive information, but the connection string contains account name and key in plaintext.

@oatsoda
Copy link
Author

oatsoda commented Jun 3, 2020

Hi @jaschrep-msft thanks for the reply.

I don't have any security worries regarding splitting out the connection string into name and key. But I have built everything to work with Connection Strings - I have it in my connectionStrings section in appSettings.json and appSettings.Development.json. I then have it set up in my App Service ARM Template to pull the connection string for the Storage Account. I can change it instead to use Account Name + Key, but that feels a bit pointless as I'm not adding anything additional - just reformatting. What if I change to Account Name + Key and later on find there is some place that requires Connection String instead?

To me, the ConnectionString is synonymous with the Account Name + Key: The two should be interchangeable - and if not then drop one of them and make us use the other so that we don't end up configuring both.

The simplest way (from my perspective - obviously you have better understanding and visibility of the technical reasons!) would be to have a constructor for StorageSharedKeyCredential which takes the Connection String. At least then I just have a single config - I don't mind too much using it in two places as I can deal with that easily using DI.

My note about creating it from BlobServiceClient was probably influenced by that fact that this was how you used to do it < v12. If there are different types of Account configured within BlobServiceClient then I can understand you can necessarily create a StorageSharedKeyCredential from it.

@satsuper
Copy link

Hi @jaschrep-msft

Please refer to the MS samples for this with a connection string in v11 here. It doesn't have to be created from a BlobServiceClient but there needs to be some way of parsing a connection string and creating a StorageSharedKeyCredential. The current approach is a clear step back from the v11 SDK.

Can I ask why the documentation on v12 SDKs is so poor? Many of the how-to links on the MS Docs page still use V11 code and from my recollection it has been more than 6 months since the GA release of these libraries. The GitHub samples don't cover the API surface either so where are developers meant to find out how to use the latest SDK?

My experience with the v12 libraries has been wasting time looking for documentation and then wasting more time figuring out how to implement something from the v11 API in v12.

For anyone else coming across this I used this workaround in the end.

static StorageSharedKeyCredential GetCredentialFromConnectionString(string connectionString)
{
	const string accountNameLabel = "AccountName";
	const string accountKeyLabel = "AccountKey";
	const string devStoreLabel = "UseDevelopmentStorage";

	const string devStoreAccountName = "devstoreaccount1";
	const string devStoreAccountKey = "Eby8vdM02xNOcqFlqUwJPLlmEtlCDXJ1OUzFT50uSRZ6IFsuFq2UVErCz4I6tq/K1SZFPTOtr/KBHBeksoGMGw==";
	const string errorMessage = "The connection string must have an AccountName and AccountKey or UseDevelopmentStorage=true";

	try
	{
		var connectionStringValues = connectionString.Split(';')
		.Select(s => s.Split(new char[] { '=' }, 2))
		.ToDictionary(s => s[0], s => s[1]);

		string accountName;
		string accountKey;
		if (connectionStringValues.TryGetValue(devStoreLabel, out var devStoreValue) && devStoreValue == "true")
		{
			accountName = devStoreAccountName;
			accountKey = devStoreAccountKey;
		}
		else
		{
			if (connectionStringValues.TryGetValue(accountNameLabel, out var accountNameValue)
			&& !string.IsNullOrWhiteSpace(accountNameValue)
			&& connectionStringValues.TryGetValue(accountKeyLabel, out var accountKeyValue)
			&& !string.IsNullOrWhiteSpace(accountKeyValue))
			{
				accountName = accountNameValue;
				accountKey = accountKeyValue;
			}
			else
			{
				throw new ArgumentException(errorMessage);
			}
		}

		return new StorageSharedKeyCredential(accountName, accountKey);
	}
	catch (IndexOutOfRangeException)
	{
		throw new ArgumentException(errorMessage);
	}
}

@jaschrep-msft
Copy link
Member

Hi @oatsoda. Apologies for letting this issue fall off my radar. Related to your ask is this issue in our language-agnostic respository. That repo documents our design guidelines that Storage and all the other libraries in this repo follow. Adoption of that request will give the storage library the tools to provide a good user experience with signing a SAS token from a client already authenticated with StorageSharedKeyCredential or a connection string.

@amnguye amnguye added feature-request This issue requires a new behavior in the product in order be resolved. and removed question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Jul 15, 2020
@jsquire jsquire added this to the Backlog milestone Jul 21, 2020
@fhurta
Copy link

fhurta commented Oct 30, 2020

Hello,
any progress with this issue?
I also found it awkward not to be able to create the StorageSharedKeyCredential from connection string or get it from already authenticated client. Both BlobServiceClient and QueueServiceClient take connection string in the constructor. You can get AccountName but not the AccountKey from them.

It would be really helpful to have some kind of consistency in the library design. I understand it might happen but this issue was reported 4 month ago..

@bobbyangers
Copy link

I think this is good feature to get. expected since it was available in v11.

@stpatrick2016
Copy link

As a workaround and instead of parsing manually can use DbConnectionStringBuilder like this:

            var conBuilder = new DbConnectionStringBuilder();
            conBuilder.ConnectionString = MyConnectionString;
            var cred = new StorageSharedKeyCredential(conBuilder["AccountName"] as string, conBuilder["AccountKey"] as string);

@echomarat
Copy link

I have ported some code from v11 to v12 and feel the api is better except regarding this issue. I have also waste a lot of time figuring out how to generate a proper blob sas uri. The code in the documentation is wrong (at least it compiles) and is not working. The only thing that make it work is @satsuper post above.

@amnguye
Copy link
Member

amnguye commented Jan 8, 2021

Sorry for not updating this thread sooner. We released the ability to generate a SAS (#15972) in the current version of the SDK and you should be able to generate a SAS from BlobServiceClient or any storage client if you initialize the client with a valid connection string. So you wouldn't have to parse the connection string for the account name and key.

BlobServiceClient service = new BlobServiceClient(connectionString);
if(service.CanGenerateSasUri)
{
    service.GenerateSasUri(permissions, expiresOn); //OR
    service.GenerateSasUri(blobSasBuilder);
}

@echomarat
Copy link

@amnguye Thanks very much this seems much cleaner. Thanks again.

@amnguye
Copy link
Member

amnguye commented Jan 11, 2021

Okay sounds good! If there's any problems regarding the new API please feel free to open a new github issue. Other than that if the new API does not resolve the original issue, please let me know and tag me. Thanks!

@amnguye amnguye closed this as completed Jan 11, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Mar 28, 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. feature-request This issue requires a new behavior in the product in order be resolved. 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)
Projects
None yet
Development

No branches or pull requests

9 participants