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

DataProtection_Azure is failing when creating a protector from Configuration file #14744

Closed
MikeAlhayek opened this issue Nov 22, 2023 · 8 comments · Fixed by #14750
Closed
Labels
Milestone

Comments

@MikeAlhayek
Copy link
Member

I have the following configuration file

public class SendbirdOptionsConfiguration : IConfigureOptions<SendbirdAppOptions>
{
    private readonly ISiteService _siteService;
    private readonly IDataProtectionProvider _dataProtectionProvider;

    public SendbirdOptionsConfiguration(
        ISiteService siteService,
        IDataProtectionProvider dataProtectionProvider
        )
    {
        _siteService = siteService;
        _dataProtectionProvider = dataProtectionProvider;
    }

    public void Configure(SendbirdAppOptions options)
    {
        var settings = _siteService.GetSiteSettingsAsync().GetAwaiter().GetResult();

        var values = settings.As<SendbirdAppOptions>();

        if (!string.IsNullOrEmpty(values.ApiToken))
        {
            options.AppId = values.AppId;
            options.RegionKey = values.RegionKey;
            options.CreatedAt = values.CreatedAt;

            var protector = _dataProtectionProvider.CreateProtector(SendbirdAppOptions.ProtectorName);

            options.ApiToken = protector.Unprotect(values.ApiToken);
        }
    }
}

The line _dataProtectionProvider.CreateProtector(SendbirdAppOptions.ProtectorName) is throwing an exception.

00||Microsoft.AspNetCore.Server.Kestrel|ERROR|Connection id "0HMVBJEKTT64P", Request id "0HMVBJEKTT64P:0000003D": An unhandled exception was thrown by the application. System.Security.Cryptography.CryptographicException: The key {key-id-in-logs} was not found in the key ring. For more information go to https://aka.ms/aspnet/dataprotectionwarning
   at Microsoft.AspNetCore.DataProtection.KeyManagement.KeyRingBasedDataProtector.UnprotectCore(Byte[] protectedData, Boolean allowOperationsOnRevokedKeys, UnprotectStatus& status)
   at Microsoft.AspNetCore.DataProtection.KeyManagement.KeyRingBasedDataProtector.Unprotect(Byte[] protectedData)
   at Microsoft.AspNetCore.DataProtection.DataProtectionCommonExtensions.Unprotect(IDataProtector protector, String protectedData)
   at CloudSolutions.Sendbird.Services.SendbirdOptionsConfiguration.Configure(SendbirdAppOptions options) in /app/Modules/CloudSolutions.Sendbird/Services/SendbirdOptionsConfiguration.cs:line 37
   at Microsoft.Extensions.Options.OptionsFactory`1.Create(String name)
   at System.Lazy`1.ViaFactory(LazyThreadSafetyMode mode)

@jtkech It seems that PR #14687 is causing this issue. Maybe the services.Initialize call is called too late (after the Configure options happens). Is there a better idea here than reverting that PR?

@jtkech
Copy link
Member

jtkech commented Nov 22, 2023

Okay, will look at it

@jtkech
Copy link
Member

jtkech commented Nov 23, 2023

@MikeAlhayek Oh yes, didn't saw this

Inside a service initializer services.Initialize(async sp => whode delegate will run at the end of the shell container creation, you can resolve services including IShellConfiguration but you can't register additional services as done line 48 services.AddDataProtection().

services.Initialize(async sp =>
{
var configuration = sp.GetRequiredService<IShellConfiguration>();
var connectionString = configuration.GetValue<string>("OrchardCore_DataProtection_Azure:ConnectionString");
if (!string.IsNullOrWhiteSpace(connectionString))
{
var containerName = await GetBlobContainerNameAsync(configuration, connectionString);
services.AddDataProtection()
.PersistKeysToAzureBlobStorage(connectionString, containerName, await GetBlobNameAsync(configuration));
}
else
{
_logger.LogCritical("No connection string was supplied for OrchardCore.DataProtection.Azure. Ensure that an application setting containing a valid Azure Storage connection string is available at `Modules:OrchardCore.DataProtection.Azure:ConnectionString`.");
}
});
}

This PR should be reverted and/or find another way, my bad I approved it ;)

@MikeAlhayek
Copy link
Member Author

Maybe we can move services.AddDataProtection() out of the initializer and somehow set the blob connection in Initializer?

@jtkech
Copy link
Member

jtkech commented Nov 23, 2023

Yes somethink like that, I will look at it tomorrow

@MikeAlhayek
Copy link
Member Author

MikeAlhayek commented Nov 23, 2023

I am not sure if this is going to work since this is called at the end.

I am calling the data protector from Configuration file (which I believe happens early on). In order for this to be properly configured, we would need to initialize it early on since this is the protection key for the shell.

Even when if that works, we would somehow need to configure this from inside the Initializer

services.Configure<KeyManagementOptions>(options =>
            {
                options.XmlRepository = new AzureBlobXmlRepository(blobRefFactory);
            });

In addition to initializing asynchronously,
it would be nice to be able to configure options also asynchronously since OC is multi tenant app. Asp.net does not support configuring options asynchronously, but would be beneficial to do that in OC since it's multi-tenant and we do reload when the settings change.

@jtkech
Copy link
Member

jtkech commented Nov 24, 2023

I will look at it when I will have time, we will tackle it this week end

@jtkech
Copy link
Member

jtkech commented Nov 24, 2023

@MikeAlhayek

Try something like this

    public class BlobInfo
    {
        public string ConnectionString { get; set; }
        public string ContainerName { get; set; }
        public string BlobName { get; set; }
    }

    public override void ConfigureServices(IServiceCollection services)
    {
        var connectionString = _configuration
            .GetValue<string>("OrchardCore_DataProtection_Azure:ConnectionString");

        if (!string.IsNullOrWhiteSpace(connectionString))
        {
            services.AddSingleton(new BlobInfo { ConnectionString = connectionString });

            services.AddDataProtection()
                .PersistKeysToAzureBlobStorage(sp =>
                {
                    var blobInfo = sp.GetRequiredService<BlobInfo>();
                    return new BlobClient(
                        blobInfo.ConnectionString,
                        blobInfo.ContainerName,
                        blobInfo.BlobName);
                });

            services.Initialize(async sp =>
            {
                var blobInfo = sp.GetRequiredService<BlobInfo>();
                var configuration = sp.GetRequiredService<IShellConfiguration>();

                blobInfo.ContainerName = await GetBlobContainerNameAsync(
                    configuration, blobInfo.ConnectionString);

                blobInfo.BlobName = await GetBlobNameAsync(configuration);
            });
        }
        else
        {
            _logger.LogCritical("No connection string was supplied for ...");
        }
    }

@MikeAlhayek
Copy link
Member Author

@jtkech that sounds like a great solution! I'll test it out tomorrow. It may be a good idea to submit a PR and I'll check it out and test it.

@MikeAlhayek MikeAlhayek added this to the 1.x milestone Dec 7, 2023
@MikeAlhayek MikeAlhayek modified the milestones: 1.x, 1.8 Jan 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants