diff --git a/src/Microsoft.AspNetCore.Authentication/AuthenticationBuilder.cs b/src/Microsoft.AspNetCore.Authentication/AuthenticationBuilder.cs index c29bdeae29..54b4818851 100644 --- a/src/Microsoft.AspNetCore.Authentication/AuthenticationBuilder.cs +++ b/src/Microsoft.AspNetCore.Authentication/AuthenticationBuilder.cs @@ -84,7 +84,7 @@ namespace Microsoft.AspNetCore.Authentication return AddScheme(authenticationScheme, displayName, configureOptions: configureOptions); } - // Used to ensure that there's always a default data protection provider + // Used to ensure that there's always a default sign in scheme that's not itself private class EnsureSignInScheme : IPostConfigureOptions where TOptions : RemoteAuthenticationOptions { private readonly AuthenticationOptions _authOptions; @@ -96,7 +96,11 @@ namespace Microsoft.AspNetCore.Authentication public void PostConfigure(string name, TOptions options) { - options.SignInScheme = options.SignInScheme ?? _authOptions.DefaultSignInScheme; + options.SignInScheme = options.SignInScheme ?? _authOptions.DefaultSignInScheme ?? _authOptions.DefaultScheme; + if (string.Equals(options.SignInScheme, name, StringComparison.Ordinal)) + { + throw new InvalidOperationException(Resources.Exception_RemoteSignInSchemeCannotBeSelf); + } } } } diff --git a/src/Microsoft.AspNetCore.Authentication/Properties/Resources.Designer.cs b/src/Microsoft.AspNetCore.Authentication/Properties/Resources.Designer.cs index 4f3f147e00..b1941a7dca 100644 --- a/src/Microsoft.AspNetCore.Authentication/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNetCore.Authentication/Properties/Resources.Designer.cs @@ -66,6 +66,20 @@ namespace Microsoft.AspNetCore.Authentication internal static string FormatException_OptionMustBeProvided(object p0) => string.Format(CultureInfo.CurrentCulture, GetString("Exception_OptionMustBeProvided"), p0); + /// + /// The SignInScheme for a remote authentication handler cannot be set to itself. If it was not explicitly set, the AuthenticationOptions.DefaultSignInScheme or DefaultScheme is used. + /// + internal static string Exception_RemoteSignInSchemeCannotBeSelf + { + get => GetString("Exception_RemoteSignInSchemeCannotBeSelf"); + } + + /// + /// The SignInScheme for a remote authentication handler cannot be set to itself. If it was not explicitly set, the AuthenticationOptions.DefaultSignInScheme or DefaultScheme is used. + /// + internal static string FormatException_RemoteSignInSchemeCannotBeSelf() + => GetString("Exception_RemoteSignInSchemeCannotBeSelf"); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/Microsoft.AspNetCore.Authentication/Resources.resx b/src/Microsoft.AspNetCore.Authentication/Resources.resx index 54d22bcc94..9e831dc74f 100644 --- a/src/Microsoft.AspNetCore.Authentication/Resources.resx +++ b/src/Microsoft.AspNetCore.Authentication/Resources.resx @@ -129,4 +129,7 @@ The '{0}' option must be provided. + + The SignInScheme for a remote authentication handler cannot be set to itself. If it was not explicitly set, the AuthenticationOptions.DefaultSignInScheme or DefaultScheme is used. + \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Authentication.Test/FacebookTests.cs b/test/Microsoft.AspNetCore.Authentication.Test/FacebookTests.cs index 75de0652e4..81373403bd 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/FacebookTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/FacebookTests.cs @@ -24,6 +24,54 @@ namespace Microsoft.AspNetCore.Authentication.Facebook { public class FacebookTests { + [Fact] + public async Task VerifySignInSchemeCannotBeSetToSelf() + { + var server = CreateServer( + app => { }, + services => services.AddAuthentication().AddFacebook(o => o.SignInScheme = FacebookDefaults.AuthenticationScheme), + context => + { + // Gross + context.ChallengeAsync("Facebook").GetAwaiter().GetResult(); + return true; + }); + var error = await Assert.ThrowsAsync(() => server.SendAsync("https://example.com/challenge")); + Assert.Contains("cannot be set to itself", error.Message); + } + + [Fact] + public async Task VerifySignInSchemeCannotBeSetToSelfUsingDefaultScheme() + { + var server = CreateServer( + app => { }, + services => services.AddAuthentication(o => o.DefaultScheme = FacebookDefaults.AuthenticationScheme).AddFacebook(), + context => + { + // Gross + context.ChallengeAsync("Facebook").GetAwaiter().GetResult(); + return true; + }); + var error = await Assert.ThrowsAsync(() => server.SendAsync("https://example.com/challenge")); + Assert.Contains("cannot be set to itself", error.Message); + } + + [Fact] + public async Task VerifySignInSchemeCannotBeSetToSelfUsingDefaultSignInScheme() + { + var server = CreateServer( + app => { }, + services => services.AddAuthentication(o => o.DefaultSignInScheme = FacebookDefaults.AuthenticationScheme).AddFacebook(), + context => + { + // Gross + context.ChallengeAsync("Facebook").GetAwaiter().GetResult(); + return true; + }); + var error = await Assert.ThrowsAsync(() => server.SendAsync("https://example.com/challenge")); + Assert.Contains("cannot be set to itself", error.Message); + } + [Fact] public async Task VerifySchemeDefaults() { diff --git a/test/Microsoft.AspNetCore.Authentication.Test/GoogleTests.cs b/test/Microsoft.AspNetCore.Authentication.Test/GoogleTests.cs index f1038bb51d..8f2cc52f91 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/GoogleTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/GoogleTests.cs @@ -15,10 +15,8 @@ using Microsoft.AspNetCore.DataProtection; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.TestHost; -using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; -using Microsoft.Extensions.Options; using Newtonsoft.Json; using Xunit; @@ -26,6 +24,19 @@ namespace Microsoft.AspNetCore.Authentication.Google { public class GoogleTests { + [Fact] + public async Task VerifySignInSchemeCannotBeSetToSelf() + { + var server = CreateServer(o => + { + o.ClientId = "Test Id"; + o.ClientSecret = "Test Secret"; + o.SignInScheme = GoogleDefaults.AuthenticationScheme; + }); + var error = await Assert.ThrowsAsync(() => server.SendAsync("https://example.com/challenge")); + Assert.Contains("cannot be set to itself", error.Message); + } + [Fact] public async Task VerifySchemeDefaults() { diff --git a/test/Microsoft.AspNetCore.Authentication.Test/MicrosoftAccountTests.cs b/test/Microsoft.AspNetCore.Authentication.Test/MicrosoftAccountTests.cs index 2e249a833a..b2854e344e 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/MicrosoftAccountTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/MicrosoftAccountTests.cs @@ -27,6 +27,19 @@ namespace Microsoft.AspNetCore.Authentication.Tests.MicrosoftAccount { public class MicrosoftAccountTests { + [Fact] + public async Task VerifySignInSchemeCannotBeSetToSelf() + { + var server = CreateServer(o => + { + o.ClientId = "Test Id"; + o.ClientSecret = "Test Secret"; + o.SignInScheme = MicrosoftAccountDefaults.AuthenticationScheme; + }); + var error = await Assert.ThrowsAsync(() => server.SendAsync("https://example.com/challenge")); + Assert.Contains("cannot be set to itself", error.Message); + } + [Fact] public async Task VerifySchemeDefaults() { diff --git a/test/Microsoft.AspNetCore.Authentication.Test/OAuthTests.cs b/test/Microsoft.AspNetCore.Authentication.Test/OAuthTests.cs index aeb313daa3..30c33eb1d7 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/OAuthTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/OAuthTests.cs @@ -16,6 +16,27 @@ namespace Microsoft.AspNetCore.Authentication.OAuth { public class OAuthTests { + [Fact] + public async Task VerifySignInSchemeCannotBeSetToSelf() + { + var server = CreateServer( + app => { }, + services => services.AddAuthentication().AddOAuth("weeblie", o => + { + o.SignInScheme = "weeblie"; + o.ClientId = "whatever"; + o.ClientSecret = "whatever"; + }), + context => + { + // REVIEW: Gross. + context.ChallengeAsync("weeblie").GetAwaiter().GetResult(); + return true; + }); + var error = await Assert.ThrowsAsync(() => server.SendAsync("https://example.com/challenge")); + Assert.Contains("cannot be set to itself", error.Message); + } + [Fact] public async Task VerifySchemeDefaults() { diff --git a/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectConfigurationTests.cs b/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectConfigurationTests.cs index 74f00c8f95..d0d1c26096 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectConfigurationTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectConfigurationTests.cs @@ -8,7 +8,6 @@ using Microsoft.AspNetCore.Authentication.OpenIdConnect; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.TestHost; -using Microsoft.AspNetCore.Testing.xunit; using Microsoft.Extensions.DependencyInjection; using Xunit; @@ -46,6 +45,20 @@ namespace Microsoft.AspNetCore.Authentication.Test.OpenIdConnect Assert.Equal(HttpStatusCode.OK, transaction.Response.StatusCode); } + [Fact] + public Task ThrowsWhenSignInSchemeIsSetToSelf() + { + return TestConfigurationException( + o => + { + o.SignInScheme = OpenIdConnectDefaults.AuthenticationScheme; + o.Authority = TestServerBuilder.DefaultAuthority; + o.ClientId = "Test Id"; + o.ClientSecret = "Test Secret"; + }, + ex => Assert.Contains("cannot be set to itself", ex.Message)); + } + [Fact] public Task ThrowsWhenClientIdIsMissing() { diff --git a/test/Microsoft.AspNetCore.Authentication.Test/TwitterTests.cs b/test/Microsoft.AspNetCore.Authentication.Test/TwitterTests.cs index 746dfee6ab..6c661af45c 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/TwitterTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/TwitterTests.cs @@ -17,6 +17,19 @@ namespace Microsoft.AspNetCore.Authentication.Twitter { public class TwitterTests { + [Fact] + public async Task VerifySignInSchemeCannotBeSetToSelf() + { + var server = CreateServer(o => + { + o.ConsumerKey = "Test Consumer Key"; + o.ConsumerSecret = "Test Consumer Secret"; + o.SignInScheme = TwitterDefaults.AuthenticationScheme; + }); + var error = await Assert.ThrowsAsync(() => server.SendAsync("https://example.com/challenge")); + Assert.Contains("cannot be set to itself", error.Message); + } + [Fact] public async Task VerifySchemeDefaults() {