From ab34bdae2e5ca11fdb1b9e549febb010a5c581da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Chalet?= Date: Wed, 1 Aug 2018 01:17:11 +0200 Subject: [PATCH] Update AddServer()/AddValidation() to throw an exception when the OpenID Connect server/OAuth validation handler are already registered and to prevent custom providers --- .../Internal/OpenIddictServerInitializer.cs | 10 ++ .../OpenIddictServerExtensions.cs | 12 +- .../OpenIddictValidationInitializer.cs | 11 ++ .../OpenIddictValidationExtensions.cs | 13 ++- .../OpenIddictServerInitializerTests.cs | 53 +++++++++ .../OpenIddictServerExtensionsTests.cs | 40 +++++++ .../OpenIddictValidationInitializerTests.cs | 103 ++++++++++++++++++ .../OpenIddictValidationExtensionsTests.cs | 41 +++++++ 8 files changed, 281 insertions(+), 2 deletions(-) create mode 100644 test/OpenIddict.Validation.Tests/Internal/OpenIddictValidationInitializerTests.cs diff --git a/src/OpenIddict.Server/Internal/OpenIddictServerInitializer.cs b/src/OpenIddict.Server/Internal/OpenIddictServerInitializer.cs index 4556ccd9..2c2e737e 100644 --- a/src/OpenIddict.Server/Internal/OpenIddictServerInitializer.cs +++ b/src/OpenIddict.Server/Internal/OpenIddictServerInitializer.cs @@ -63,6 +63,16 @@ namespace OpenIddict.Server throw new InvalidOperationException("A random number generator must be registered."); } + if (options.ProviderType == null || options.ProviderType != typeof(OpenIddictServerProvider)) + { + throw new InvalidOperationException(new StringBuilder() + .AppendLine("OpenIddict can only be used with its built-in server provider.") + .AppendLine("This error may indicate that 'OpenIddictServerOptions.ProviderType' was manually set.") + .Append("To execute custom request handling logic, consider registering an event handler using ") + .Append("the generic 'services.AddOpenIddict().AddServer().AddEventHandler()' method.") + .ToString()); + } + // When no distributed cache has been registered in the options, // try to resolve it from the dependency injection container. if (options.Cache == null) diff --git a/src/OpenIddict.Server/OpenIddictServerExtensions.cs b/src/OpenIddict.Server/OpenIddictServerExtensions.cs index 5c515731..fff29084 100644 --- a/src/OpenIddict.Server/OpenIddictServerExtensions.cs +++ b/src/OpenIddict.Server/OpenIddictServerExtensions.cs @@ -70,8 +70,18 @@ namespace Microsoft.Extensions.DependencyInjection { // Note: this method is guaranteed to be idempotent. To prevent multiple schemes from being // registered (which would result in an exception being thrown), a manual check is made here. - if (options.SchemeMap.ContainsKey(OpenIddictServerDefaults.AuthenticationScheme)) + if (options.SchemeMap.TryGetValue(OpenIddictServerDefaults.AuthenticationScheme, out var handler)) { + // If the handler type doesn't correspond to the OpenIddict handler, throw an exception. + if (handler.HandlerType != typeof(OpenIddictServerHandler)) + { + throw new InvalidOperationException(new StringBuilder() + .AppendLine("The OpenIddict server handler cannot be registered as an authentication scheme.") + .AppendLine("This may indicate that an instance of the OpenID Connect server was registered.") + .Append("Make sure that 'services.AddAuthentication().AddOpenIdConnectServer()' is not used.") + .ToString()); + } + return; } diff --git a/src/OpenIddict.Validation/Internal/OpenIddictValidationInitializer.cs b/src/OpenIddict.Validation/Internal/OpenIddictValidationInitializer.cs index ab3e5b2c..bedfea03 100644 --- a/src/OpenIddict.Validation/Internal/OpenIddictValidationInitializer.cs +++ b/src/OpenIddict.Validation/Internal/OpenIddictValidationInitializer.cs @@ -6,6 +6,7 @@ using System; using System.ComponentModel; +using System.Text; using JetBrains.Annotations; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.DataProtection; @@ -48,6 +49,16 @@ namespace OpenIddict.Validation throw new ArgumentException("The options instance name cannot be null or empty.", nameof(name)); } + if (options.EventsType == null || options.EventsType != typeof(OpenIddictValidationProvider)) + { + throw new InvalidOperationException(new StringBuilder() + .AppendLine("OpenIddict can only be used with its built-in validation provider.") + .AppendLine("This error may indicate that 'OpenIddictValidationOptions.EventsType' was manually set.") + .Append("To execute custom request handling logic, consider registering an event handler using ") + .Append("the generic 'services.AddOpenIddict().AddValidation().AddEventHandler()' method.") + .ToString()); + } + if (options.DataProtectionProvider == null) { options.DataProtectionProvider = _dataProtectionProvider; diff --git a/src/OpenIddict.Validation/OpenIddictValidationExtensions.cs b/src/OpenIddict.Validation/OpenIddictValidationExtensions.cs index 8c74f88e..80d7fc6c 100644 --- a/src/OpenIddict.Validation/OpenIddictValidationExtensions.cs +++ b/src/OpenIddict.Validation/OpenIddictValidationExtensions.cs @@ -5,6 +5,7 @@ */ using System; +using System.Text; using AspNet.Security.OAuth.Validation; using JetBrains.Annotations; using Microsoft.AspNetCore.Authentication; @@ -53,8 +54,18 @@ namespace Microsoft.Extensions.DependencyInjection { // Note: this method is guaranteed to be idempotent. To prevent multiple schemes from being // registered (which would result in an exception being thrown), a manual check is made here. - if (options.SchemeMap.ContainsKey(OpenIddictValidationDefaults.AuthenticationScheme)) + if (options.SchemeMap.TryGetValue(OpenIddictValidationDefaults.AuthenticationScheme, out var handler)) { + // If the handler type doesn't correspond to the OpenIddict handler, throw an exception. + if (handler.HandlerType != typeof(OpenIddictValidationHandler)) + { + throw new InvalidOperationException(new StringBuilder() + .AppendLine("The OpenIddict validation handler cannot be registered as an authentication scheme.") + .AppendLine("This may indicate that an instance of the OAuth validation handler was registered.") + .Append("Make sure that 'services.AddAuthentication().AddOAuthValidation()' is not used.") + .ToString()); + } + return; } diff --git a/test/OpenIddict.Server.Tests/Internal/OpenIddictServerInitializerTests.cs b/test/OpenIddict.Server.Tests/Internal/OpenIddictServerInitializerTests.cs index 72eacfbd..5f9385f5 100644 --- a/test/OpenIddict.Server.Tests/Internal/OpenIddictServerInitializerTests.cs +++ b/test/OpenIddict.Server.Tests/Internal/OpenIddictServerInitializerTests.cs @@ -9,6 +9,7 @@ using System.Text; using System.Threading.Tasks; using AspNet.Security.OpenIdConnect.Client; using AspNet.Security.OpenIdConnect.Primitives; +using AspNet.Security.OpenIdConnect.Server; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; @@ -43,6 +44,58 @@ namespace OpenIddict.Server.Tests Assert.Equal("A random number generator must be registered.", exception.Message); } + [Fact] + public async Task PostConfigure_ThrowsAnExceptionWhenProviderTypeIsNull() + { + // Arrange + var server = CreateAuthorizationServer(builder => + { + builder.Configure(options => options.ProviderType = null); + }); + + var client = new OpenIdConnectClient(server.CreateClient()); + + // Act and assert + var exception = await Assert.ThrowsAsync(delegate + { + return client.GetAsync("/"); + }); + + // Assert + Assert.Equal(new StringBuilder() + .AppendLine("OpenIddict can only be used with its built-in server provider.") + .AppendLine("This error may indicate that 'OpenIddictServerOptions.ProviderType' was manually set.") + .Append("To execute custom request handling logic, consider registering an event handler using ") + .Append("the generic 'services.AddOpenIddict().AddServer().AddEventHandler()' method.") + .ToString(), exception.Message); + } + + [Fact] + public async Task PostConfigure_ThrowsAnExceptionWhenProviderTypeIsIncompatible() + { + // Arrange + var server = CreateAuthorizationServer(builder => + { + builder.Configure(options => options.ProviderType = typeof(OpenIdConnectServerProvider)); + }); + + var client = new OpenIdConnectClient(server.CreateClient()); + + // Act and assert + var exception = await Assert.ThrowsAsync(delegate + { + return client.GetAsync("/"); + }); + + // Assert + Assert.Equal(new StringBuilder() + .AppendLine("OpenIddict can only be used with its built-in server provider.") + .AppendLine("This error may indicate that 'OpenIddictServerOptions.ProviderType' was manually set.") + .Append("To execute custom request handling logic, consider registering an event handler using ") + .Append("the generic 'services.AddOpenIddict().AddServer().AddEventHandler()' method.") + .ToString(), exception.Message); + } + [Fact] public async Task PostConfigure_ThrowsAnExceptionWhenNoFlowIsEnabled() { diff --git a/test/OpenIddict.Server.Tests/OpenIddictServerExtensionsTests.cs b/test/OpenIddict.Server.Tests/OpenIddictServerExtensionsTests.cs index 7968897a..baed90f9 100644 --- a/test/OpenIddict.Server.Tests/OpenIddictServerExtensionsTests.cs +++ b/test/OpenIddict.Server.Tests/OpenIddictServerExtensionsTests.cs @@ -204,5 +204,45 @@ namespace OpenIddict.Server.Tests Assert.Contains(options.Schemes, scheme => scheme.Name == OpenIddictServerDefaults.AuthenticationScheme && scheme.HandlerType == typeof(OpenIddictServerHandler)); } + + [Fact] + public void AddServer_ThrowsAnExceptionWhenSchemeIsAlreadyRegisteredWithDifferentHandlerType() + { + // Arrange + var services = new ServiceCollection(); + services.AddAuthentication() + .AddOpenIdConnectServer(); + + var builder = new OpenIddictBuilder(services); + + // Act + builder.AddServer(); + + // Assert + var provider = services.BuildServiceProvider(); + var exception = Assert.Throws(delegate + { + return provider.GetRequiredService>().Value; + }); + + Assert.Equal(new StringBuilder() + .AppendLine("The OpenIddict server handler cannot be registered as an authentication scheme.") + .AppendLine("This may indicate that an instance of the OpenID Connect server was registered.") + .Append("Make sure that 'services.AddAuthentication().AddOpenIdConnectServer()' is not used.") + .ToString(), exception.Message); + } + + [Fact] + public void AddServer_CanBeSafelyInvokedMultipleTimes() + { + // Arrange + var services = new ServiceCollection(); + var builder = new OpenIddictBuilder(services); + + // Act and assert + builder.AddServer(); + builder.AddServer(); + builder.AddServer(); + } } } diff --git a/test/OpenIddict.Validation.Tests/Internal/OpenIddictValidationInitializerTests.cs b/test/OpenIddict.Validation.Tests/Internal/OpenIddictValidationInitializerTests.cs new file mode 100644 index 00000000..3710fe51 --- /dev/null +++ b/test/OpenIddict.Validation.Tests/Internal/OpenIddictValidationInitializerTests.cs @@ -0,0 +1,103 @@ +/* + * Licensed under the Apache License, Version 2.0 (http://www.apache.org/licenses/LICENSE-2.0) + * See https://github.com/openiddict/openiddict-core for more information concerning + * the license and the contributors participating to this project. + */ + +using System; +using System.Text; +using System.Threading.Tasks; +using AspNet.Security.OAuth.Validation; +using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Xunit; + +namespace OpenIddict.Validation.Tests +{ + public class OpenIddictValidationInitializerTests + { + [Fact] + public async Task PostConfigure_ThrowsAnExceptionWhenEventsTypeIsNull() + { + // Arrange + var server = CreateResourceServer(builder => + { + builder.Configure(options => options.EventsType = null); + }); + + var client = server.CreateClient(); + + // Act and assert + var exception = await Assert.ThrowsAsync(delegate + { + return client.GetAsync("/"); + }); + + // Assert + Assert.Equal(new StringBuilder() + .AppendLine("OpenIddict can only be used with its built-in validation provider.") + .AppendLine("This error may indicate that 'OpenIddictValidationOptions.EventsType' was manually set.") + .Append("To execute custom request handling logic, consider registering an event handler using ") + .Append("the generic 'services.AddOpenIddict().AddValidation().AddEventHandler()' method.") + .ToString(), exception.Message); + } + + [Fact] + public async Task PostConfigure_ThrowsAnExceptionWhenEventsTypeIsIncompatible() + { + // Arrange + var server = CreateResourceServer(builder => + { + builder.Configure(options => options.EventsType = typeof(OAuthValidationEvents)); + }); + + var client = server.CreateClient(); + + // Act and assert + var exception = await Assert.ThrowsAsync(delegate + { + return client.GetAsync("/"); + }); + + // Assert + Assert.Equal(new StringBuilder() + .AppendLine("OpenIddict can only be used with its built-in validation provider.") + .AppendLine("This error may indicate that 'OpenIddictValidationOptions.EventsType' was manually set.") + .Append("To execute custom request handling logic, consider registering an event handler using ") + .Append("the generic 'services.AddOpenIddict().AddValidation().AddEventHandler()' method.") + .ToString(), exception.Message); + } + + private static TestServer CreateResourceServer(Action configuration = null) + { + var builder = new WebHostBuilder(); + + builder.UseEnvironment("Testing"); + + builder.ConfigureLogging(options => options.AddDebug()); + + builder.ConfigureServices(services => + { + services.AddAuthentication(); + services.AddOptions(); + services.AddDistributedMemoryCache(); + + services.AddOpenIddict() + .AddValidation(options => configuration?.Invoke(options)); + }); + + builder.Configure(app => + { + app.UseAuthentication(); + + app.Run(context => context.ChallengeAsync(OpenIddictValidationDefaults.AuthenticationScheme)); + }); + + return new TestServer(builder); + } + } +} diff --git a/test/OpenIddict.Validation.Tests/OpenIddictValidationExtensionsTests.cs b/test/OpenIddict.Validation.Tests/OpenIddictValidationExtensionsTests.cs index eb016079..7fde8265 100644 --- a/test/OpenIddict.Validation.Tests/OpenIddictValidationExtensionsTests.cs +++ b/test/OpenIddict.Validation.Tests/OpenIddictValidationExtensionsTests.cs @@ -5,6 +5,7 @@ */ using System; +using System.Text; using AspNet.Security.OAuth.Validation; using Microsoft.AspNetCore.Authentication; using Microsoft.Extensions.DependencyInjection; @@ -165,5 +166,45 @@ namespace OpenIddict.Validation.Tests Assert.Contains(options.Schemes, scheme => scheme.Name == OpenIddictValidationDefaults.AuthenticationScheme && scheme.HandlerType == typeof(OpenIddictValidationHandler)); } + + [Fact] + public void AddValidation_ThrowsAnExceptionWhenSchemeIsAlreadyRegisteredWithDifferentHandlerType() + { + // Arrange + var services = new ServiceCollection(); + services.AddAuthentication() + .AddOAuthValidation(); + + var builder = new OpenIddictBuilder(services); + + // Act + builder.AddValidation(); + + // Assert + var provider = services.BuildServiceProvider(); + var exception = Assert.Throws(delegate + { + return provider.GetRequiredService>().Value; + }); + + Assert.Equal(new StringBuilder() + .AppendLine("The OpenIddict validation handler cannot be registered as an authentication scheme.") + .AppendLine("This may indicate that an instance of the OAuth validation handler was registered.") + .Append("Make sure that 'services.AddAuthentication().AddOAuthValidation()' is not used.") + .ToString(), exception.Message); + } + + [Fact] + public void AddValidation_CanBeSafelyInvokedMultipleTimes() + { + // Arrange + var services = new ServiceCollection(); + var builder = new OpenIddictBuilder(services); + + // Act and assert + builder.AddValidation(); + builder.AddValidation(); + builder.AddValidation(); + } } }