Support of multiple schemas and IApiKeyProvider

[maartenmensink]

Hi,

Thanks for creating this library. I was testing the library and came across the following.

I am trying to add a second schema but due to a registration issue in the ApiKeyExtension the second provider overrides the first provider.

services.AddAuthentication(ApiKeyDefaults.AuthenticationScheme)
	.AddApiKeyInHeader<ProviderOne>(options =>
	{
		options.Realm = "Realm 1";
		options.KeyName = "X-API-KEY";
	})
	.AddApiKeyInHeader<ProviderTwo>("SCHEMA2", options =>
	{
		options.Realm = "Realm 2";
		options.KeyName = "X-API-KEY";
	});

ApiKeyExtensions on line 384

private static AuthenticationBuilder AddApiKey<TApiKeyProvider, TApiKeyHandler>(this AuthenticationBuilder builder, string authenticationScheme, string displayName, Action<ApiKeyOptions> configureOptions)
	where TApiKeyProvider : class, IApiKeyProvider
	where TApiKeyHandler : AuthenticationHandler<ApiKeyOptions>
{
	// Adds implementation of IApiKeyProvider to the dependency container.
	builder.Services.AddTransient<IApiKeyProvider, TApiKeyProvider>(); //I think that this overrides the registration make 
	builder.Services.Configure<ApiKeyOptions>(
		authenticationScheme,
		o => o.ApiKeyProviderType = typeof(TApiKeyProvider)
	);

	// Adds post configure options to the pipeline.
	builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton<IPostConfigureOptions<ApiKeyOptions>, ApiKeyPostConfigureOptions>());

	// Adds api key authentication scheme to the pipeline.
	return builder.AddScheme<ApiKeyOptions, TApiKeyHandler>(authenticationScheme, displayName, configureOptions);

}

This will cause ApiKeyHandlerBase:L193 to always return a instance to return. This will prevent it from reaching if (apiKeyProvider == null) and resolving a provider based on the type in the options
apiKeyProvider = Context.RequestServices.GetService(Options.ApiKeyProviderType) as IApiKeyProvider;

private async Task<IApiKey> ValidateUsingApiKeyProviderAsync(string apiKey)
{
	var apiKeyProvider = Context.RequestServices.GetService<IApiKeyProvider>();
	if (apiKeyProvider == null)
	{
		if (Options.ApiKeyProviderType != null)
		{
			apiKeyProvider = Context.RequestServices.GetService(Options.ApiKeyProviderType) as IApiKeyProvider;
		}

		if (apiKeyProvider == null)
		{
			throw new InvalidOperationException($"Either {nameof(Options.Events.OnValidateKey)} delegate on configure options {nameof(Options.Events)} should be set or an implementaion of {nameof(IApiKeyProvider)} should be registered in the dependency container.");
		}
	}

	return await apiKeyProvider.ProvideAsync(apiKey).ConfigureAwait(false);
}

[mihirdilip]

Hi @maartenmensink, I really appreciate for taking your time out to test the library and raising issues. I have pushed changes to the master branch. I hope this will fix the issue. Please let me know if it passes your tests and you are happy with the implementation. If you are using NuGet package and not source code from github then I can push it to NuGet for you to try it out.

private async Task<IApiKey> ValidateUsingApiKeyProviderAsync(string apiKey)
{
    IApiKeyProvider apiKeyProvider = null;
    if (Options.ApiKeyProviderType != null)
    {
        apiKeyProvider = ActivatorUtilities.GetServiceOrCreateInstance(Context.RequestServices, Options.ApiKeyProviderType) as IApiKeyProvider;
    }

    if (apiKeyProvider == null)
    {
        throw new InvalidOperationException($"Either {nameof(Options.Events.OnValidateKey)} delegate on configure options {nameof(Options.Events)} should be set or use an extention method with type parameter of type {nameof(IApiKeyProvider)}.");
    }

    try
    {
        return await apiKeyProvider.ProvideAsync(apiKey).ConfigureAwait(false);
    }
    finally
    {
        if (apiKeyProvider is IDisposable disposableApiKeyProvider)
        {
            disposableApiKeyProvider.Dispose();
        }
    }
}

Please let me know you thoughts.

Regards,
Mihir

[maartenmensink]

You are welcome a nuget would be great. I can test the fix first if you would like.

[mihirdilip]

@maartenmensink I have uploaded it on NuGet. Version is 3.1.1

[mihirdilip]

Hi @maartenmensink, has the new release fixed the issue for you?

[mihirdilip]

Assuming it is fixed, I am closing this issue.

[maartenmensink]

Sorry havent been able to find the time to test the fix but looking at the commit it will work!