umbraco / Umbraco.StorageProviders

MIT License
29 stars 21 forks source link

[Feature Request] Authorize access to blob data with managed identities for Azure resources #43

Closed olibos closed 1 month ago

olibos commented 2 years ago

Hello 👋

First of all:

Great work on this library! 🎉

Context:

With the release of Umbraco 10 (and NPOCO 5 support) we have completly remove the passwords from our database connection string. It could be great if we can do the same with the azure storage part.

Details:

https://docs.microsoft.com/en-us/azure/storage/blobs/authorize-managed-identity

If you're interested, I can try on my side and make a PR 😉

Have a nice day,

Olivier

RBerntsen commented 2 years ago

Ah, was just looking for exactly the same. Not an expert in this (haven't worked with it before) but according to what documentation I can find it should be something like this:

Today the BlobContainerClient is created like this: _containerClient = new BlobContainerClient(options.ConnectionString, options.ContainerName);

The constructor should also have a possibility to create it from an identity: _containerClient = new BlobContainerClient(new Uri(uri), new DefaultAzureCredential()))

I was looking at the example here: https://docs.microsoft.com/en-us/dotnet/api/azure.identity.defaultazurecredential?view=azure-dotnet#examples

It also provides info on how the DefaultAzureCredential works.

olibos commented 2 years ago

Hello @RBerntsen ,

Here is the current suggested way to add it: https://github.com/umbraco/Umbraco.StorageProviders/issues/21#issuecomment-1076463177

public static IUmbracoBuilder AddAzureBlobMediaFileSystem(this IUmbracoBuilder builder, Uri blobContainerUri, bool useAzureBlobImageCache = true)
{
    if (builder == null) throw new ArgumentNullException(nameof(builder));

    builder.SetMediaFileSystem(provider =>
    {
        var globalSettingsOptions = provider.GetRequiredService<IOptions<GlobalSettings>>();
        var hostingEnvironment = provider.GetRequiredService<IHostingEnvironment>();

        var rootUrl = hostingEnvironment.ToAbsolute(globalSettingsOptions.Value.UmbracoMediaPath);
        var blobContainerClient = new BlobContainerClient(blobContainerUri, new DefaultAzureCredential());
        var ioHelper = provider.GetRequiredService<IOHelper>();
        var contentTypeProvider = new FileExtensionContentTypeProvider();

        return new AzureBlobFileSystem(rootUrl, blobContainerClient, ioHelper, contentTypeProvider);
    });

    // ImageSharp image cache
    if (useAzureBlobImageCache)
    {
        builder.Services.AddUnique<IImageCache>(_ =>
        {
            var blobContainerClient = new BlobContainerClient(blobContainerUri, new DefaultAzureCredential());

            return new AzureBlobFileSystemImageCache(blobContainerClient);
        });
    }

    return builder;
}

I hope we can find a more built-in way based on configuration / environment variables 😉

RBerntsen commented 2 years ago

@olibos That's cool, thanks for sharing!

olibos commented 2 years ago

You're welcome, but the credits goes to @ronaldbarendse 😉

Prebiusta commented 1 year ago

Is there any update on this feature? I think it could significantly improve the supported security for the library.

ronaldbarendse commented 8 months ago

I've created a PR that should allow you to use managed identities without having to add the Azure.Identity dependency to the Umbraco.StorageProviders.AzureBlob package or creating an additional supporting package, see #65.

It would be great if anyone can test this, so I've pushed the PR preview build to our nightly MyGet feed: https://www.myget.org/feed/umbraconightly/package/nuget/Umbraco.StorageProviders.AzureBlob/13.0.2--alpha.preview.5.geff204d.

You can then configure the media file system like this:

using Azure.Identity;
using Azure.Storage.Blobs;
using Umbraco.Cms.Core.Composing;
using Umbraco.StorageProviders.AzureBlob.IO;

internal sealed class AzureBlobFileSystemComposer : IComposer
{
    public void Compose(IUmbracoBuilder builder)
        => builder.AddAzureBlobMediaFileSystem(options =>
        {
            options.ConnectionString = "https://[storage-account].blob.core.windows.net";
            options.ContainerName = "media";
            options.TryCreateBlobContainerClientUsingUri(uri => new BlobContainerClient(uri, new DefaultAzureCredential()));
        });
}
ctolkien commented 4 months ago

Ideally the AzureBlobFileSystem should just defer to DI to get an instance of the BlobContainerClient. If there isn't one registered in the container by the time AddAzureBlobMediaFileSystem is called, it should then add it's own one with the defaults configured.

There are statics like https://github.com/umbraco/Umbraco.StorageProviders/blob/3eec2adf53371d73abfbaf08bdfaeb2120dc87c2/src/Umbraco.StorageProviders.AzureBlob/IO/AzureBlobFileSystem.cs#L74 which also new up instances and cannot be overwritten.

ronaldbarendse commented 1 month ago

There are statics like

https://github.com/umbraco/Umbraco.StorageProviders/blob/3eec2adf53371d73abfbaf08bdfaeb2120dc87c2/src/Umbraco.StorageProviders.AzureBlob/IO/AzureBlobFileSystem.cs#L74

which also new up instances and cannot be overwritten.

This static method now defers creating the BlobContainerClient to the configured factory of the AzureBlobFileSystemOptions: https://github.com/umbraco/Umbraco.StorageProviders/blob/3fb634500016485fd44d1fb752abd3191dd9e680/src/Umbraco.StorageProviders.AzureBlob/IO/AzureBlobFileSystem.cs#L79-L84

I've merged PR https://github.com/umbraco/Umbraco.StorageProviders/pull/65 and will prepare new releases for v13+ (including a v15 RC).

ctolkien commented 1 month ago

From what I can tell, this PR, whilst a step in the right direction in supporting managed identities, doesn't actually solve the issue for us - that being we already have a BlobContainerClient registered with the DI container that cannot be reused here.