Support azure blob storage auth with managed identity.

[mariojsnunes]

Fixes: #12639

TODO:

• Add docs

To test:

1. Configure a Storage Account on Azure:
   a. Create a Storage Account
   b. Go to Configuration and disable "Allow storage account key access"
   c. Go to Access Control (IAM) and add the role Storage Blob Data Contributor to the account that you use on Visual Studio (Tools > Options > Azure Service Authentication > Account Selection).
2. Update the following files on OrchardCore.Cms.Web:

Program.cs

builder.Services.AddAzureClients(clientBuilder =>
{
    clientBuilder.AddBlobServiceClient(new Uri("https://<account-name>.blob.core.windows.net")).WithName("ShellsBlobStorage");
    clientBuilder.UseCredential(new DefaultAzureCredential());
});

builder.Services
    .AddOrchardCms()
    .ConfigureServices(x => x.AddAzureClients(clientBuilder =>
    {
        clientBuilder.AddBlobServiceClient(new Uri("https://<account-name>.blob.core.windows.net")).WithName("MediaBlobStorage");
        clientBuilder.AddBlobServiceClient(new Uri("https://<account-name>.blob.core.windows.net")).WithName("DataProtectionBlobStorage");
        clientBuilder.UseCredential(new DefaultAzureCredential());
    }), -1)
    .AddAzureShellsConfiguration()
    .AddSetupFeatures("OrchardCore.AutoSetup");

appsettings.json

 "OrchardCore_DataProtection_Azure": {
   "ConnectionString": "", // Set to your Azure Storage account connection string.
   "AzureClientName": "DataProtectionBlobStorage", // Set to your Azure Client name.
   "ContainerName": "dataprotection", // Default to dataprotection. Templatable, refer docs.
   "BlobName": "", // Optional, defaults to Sites/tenant_name/DataProtectionKeys.xml. Templatable, refer docs.
   "CreateContainer": true // Creates the container during app startup if it does not already exist.
 },
  "OrchardCore_Media_Azure": {
   "ConnectionString": "", // Set to your Azure Storage account connection string.
   "AzureClientName": "MediaBlobStorage", // Set to your Azure Client name.
   "ContainerName": "testoc", // Set to the Azure Blob container name. Templatable, refer docs.
   "BasePath": "media", // Optionally, set to a path to store media in a subdirectory inside your container. Templatable, refer docs.
   "CreateContainer": true, // Activates an event to create the container if it does not already exist.
   "RemoveContainer": true // Whether the 'Container' is deleted if the tenant is removed, false by default.
 },
  "OrchardCore_Shells_Azure": {
   "ConnectionString": "", // Set to your Azure Storage account connection string.
   "AzureClientName": "ShellsBlobStorage", // Set to your Azure Client name.
   "ContainerName": "hostcontainer", // Set to the Azure Blob container name.
   //"BasePath": "some/base/path", // Optionally, set to a subdirectory inside your container.
   "MigrateFromFiles": true // Optionally, enable to migrate existing App_Data files to Blob automatically.
 },

[mariojsnunes]

I'd add a new ManagedIdentityClientId property to MediaBlobStorageOptions and possibly BlobShellStorageOptions. Not BlobStorageOptions because the Azure Image Cache of ImageSharp doesn't support it

@Piedone I know you said this, but how would you instantiate the client at BlobFileStore without adding those properties to BlobStorageOptions? Maybe with a different approach?

[mariojsnunes]

Some of the code is based on AzureAI module as per @MikeAlhayek suggestion.

[mariojsnunes]

A few topics to be worked on:

1. I realize now that would be better to do this with .AddAzureClients and IAzureClientFactory like the original PR. But where would .AddAzureClients be added? To projects that reference OrchardCore (and we mention in on the docs)?
2. Should this be done for DataProtection too?

[Piedone]

Please mark this PR ready for review, i.e. not draft, once it's not a WIP.

Yeah, indeed, you'll need to extend BlobStorageOptions for BlobFileStore. Under #15028 this will be an issue, since currently, it can be nicely used by all Blob storage-using code. For the two features in Media.Azure, an intermediate base class makes both of the have the exact same configuration options.

Since as mentioned, the Azure ImageSharp Image Cache only supports key-based configuration, this inheritance chain will need to be broken and the current classes copied, or some runtime exception added to make it fail if you try to configure managed identity for ImageSharp. I'm not sure which one is better (both are bad), but if that PR gets merged first, please implement something for this.

[mariojsnunes]

I've added testing instructions to the first comment.

I'm not sure which one is better (both are bad)

I think we can simply not use the "AzureClientName" config on whichever module doesn't support it.
The main difference from the original PR is that I didn't create a Factory for BlobFileStore, so each module has full control over it.

[Piedone]

OK then. So, this is ready then, minus documentation?

[mariojsnunes]

OK then. So, this is ready then, minus documentation?

Yes, but I'd appreciate if someone else could test in case I overlooked something.

• Media
• DataProtection
• Shells

[Piedone]

We need to support multi-tenancy and configuration providers with every feature. So, this should also work with configuration coming solely from IShellConfiguration, thus supporting hierarchical configuration; we shouldn't have to configure anything in Program (having that option is good, but that being required doesn't match how Orchard operates).

I'm talking about adding URLs there. It's fine to have something like AddAzureManagedIdentity() to opt-in Iike AddAzureShellsConfiguration(), but the rest should happen internally and ultimately from IShellConfiguration.

[github-actions]

This pull request has merge conflicts. Please resolve those before requesting a review.

[github-actions]

It seems that this pull request didn't really move for quite a while. Is this something you'd like to revisit any time soon or should we close? Please comment if you'd like to pick it up.

[hishamco]

Please fix the conflict

[Piedone]

Hisham, you really don't need to comment these. This is already added by the bot, and such comments just needlessly keep otherwise abandoned PRs alive.

[hishamco]

Thanks for let me know that, I didn't know that this bot is smart enoght 😜

[mariojsnunes]

Sorry I haven't had the time to pick this up again. Hopefuly next week.
From what I saw the conflicts are not easily resolved, better to just redo.

I also remember I wasn't sure which changes were needed for the hierarchical configuration to work.

[MikeAlhayek]

for instructions on how to resolve the merge conflicts due to #16572 please follow the step listed in this comment.

[github-actions]

It seems that this pull request didn't really move for quite a while. Is this something you'd like to revisit any time soon or should we close? Please comment if you'd like to pick it up.

[mariojsnunes]

can be closed. I haven't managed to find the time.