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

Ability to connect to Azure Blob storage with AAD authentication/managed identity #12639

Open
lassiko opened this issue Oct 14, 2022 · 8 comments · May be fixed by #15753
Open

Ability to connect to Azure Blob storage with AAD authentication/managed identity #12639

lassiko opened this issue Oct 14, 2022 · 8 comments · May be fixed by #15753
Milestone

Comments

@lassiko
Copy link

lassiko commented Oct 14, 2022

Currently, one has to configure Azure storage with connection string containing the AccountKey, which implies the need to expose it in appsettings.json or Azure App Service app settings.

To enhance security, it would be good to be able to use only the storage URL and use AAD authentication and App service's managed identity, as one can already do in OC with Azure SQL database.

@sebastienros
Copy link
Member

Would you be able to provide a PR for that?

@sebastienros sebastienros added this to the 1.x milestone Oct 20, 2022
@carlin-q-scott
Copy link

Just some notes for whoever implements this:

  1. Add ContainerUri to OrchardCore_Media_Azure options
  2. Construct BlobContainerClass using this method:
    new BlobServiceClient(
            new Uri(blobOptions.ContainerUri),
            new DefaultAzureCredential())

That's it AFAIK based on the documentation.

@rikbosch
Copy link

Just some notes for whoever implements this:

  1. Add ContainerUri to OrchardCore_Media_Azure options
  2. Construct BlobContainerClass using this method:
    new BlobServiceClient(
            new Uri(blobOptions.ContainerUri),
            new DefaultAzureCredential())

That's it AFAIK based on the documentation.

What about configuring BlobClientOptions like retries etc?

Maybe we should be able to specify the 'name' of the blobclient to use and use that in combination with Microsoft.Extensions.Azure :

// first configure the azure clients 
builder.Services.AddAzureClients(az=>
{
    // Establish the global defaults
    builder.ConfigureDefaults(Configuration.GetSection("AzureDefaults"));
    builder.UseCredential(new DefaultAzureCredential());


      // A named storage client with a different custom retry policy
    builder.AddBlobServiceClient(Configuration.GetSection("CustomStorage"))
      .WithName("CustomStorage")
      .ConfigureOptions(options => {
        options.Retry.Mode = Azure.Core.RetryMode.Exponential;
        options.Retry.MaxRetries = 5;
        options.Retry.MaxDelay = TimeSpan.FromSections(120);
      });
});

// and then later configure this:
//....

builder.Services.AddOrchardCms(orchard=>
{
     orchard.AddAzureShellsConfiguration(); // 
});

with the following configuration in appsettings.json (or any other configuration provider)

  "OrchardCore": {       
    "OrchardCore_Shells_Azure": {
      "ClientName" : "CustomStorage" // <<<< this is the important part
      "ContainerName": "some-container", // Set to the Azure Blob container name.
      "BasePath": "shells" // Optionally, set to a subdirectory inside your container.    
    }
  }

these examples are partially taken from:

https://devblogs.microsoft.com/azure-sdk/best-practices-for-using-azure-sdk-with-asp-net-core/

rikbosch added a commit to rikbosch/OrchardCore that referenced this issue Nov 23, 2022
This allows to use registered clients from Azure.Configuration.Extensions  (and DefaultAzureCredential )or
specify a connectionstring
@carlin-q-scott
Copy link

@rikbosch That works too. It's a little more complicated to set up for us developers but I like the flexibility and the fact that you already implemented it 😄

@rikbosch
Copy link

rikbosch commented Nov 24, 2022

I Updated the PR to fallback to the Default BlobServiceClient.

It allows for more complexity but could be as simple as:

// first configure the azure clients 
builder.Services.AddAzureClients(az=>
{
    builder.UseCredential(new DefaultAzureCredential());

      // a "Default" blobservice client
    builder.AddBlobServiceClient("storageUri");
});
  "OrchardCore": {       
    "OrchardCore_Shells_Azure": {
       // no ConnectionString or BlobServiceName set, so it will try to use the Default registered BlobServiceClient
      "ContainerName": "some-container", // Set to the Azure Blob container name.
      "BasePath": "shells" // Optionally, set to a subdirectory inside your container.    
    }
  }

@rikbosch
Copy link

Also, for performance reasons, a DefaultAzureCredential should be cached, as per the docs:

Acquired tokens are cached by the credential instance. Token lifetime and refreshing is handled automatically. Where possible, reuse credential instances to optimize cache effectiveness.

https://learn.microsoft.com/en-us/dotnet/api/azure.identity.defaultazurecredential.gettokenasync?view=azure-dotnet#definition

@Piedone
Copy link
Member

Piedone commented Apr 8, 2024

Duplicated by #15663. See #15663 (comment) for implementation suggestions.

@MikeAlhayek
Copy link
Member

I did similar implementation using AzureAI module. Similar logic can be done here. We can also provide the configuration via configuration provider of via UI. The user can select the default settings or provide their own settings. The settings could be Default, API key or other supported methods.

Enable the AzureAI search module to evaluate the current setup in OC for this scenario

@mariojsnunes mariojsnunes linked a pull request Apr 14, 2024 that will close this issue
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment