From 02cd997e3258a4e928a2cf1cf28f7ccae66ec635 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Tue, 10 Oct 2017 13:51:07 -0700 Subject: [PATCH] Add Validate(scheme) and use for RemoteSignInScheme not self validation --- .../AuthenticationBuilder.cs | 4 ---- .../AuthenticationHandler.cs | 2 +- .../AuthenticationSchemeOptions.cs | 11 ++++++++--- .../RemoteAuthenticationOptions.cs | 13 +++++++++++++ .../FacebookTests.cs | 16 +++++++++++++--- .../OAuthTests.cs | 3 +++ 6 files changed, 38 insertions(+), 11 deletions(-) diff --git a/src/Microsoft.AspNetCore.Authentication/AuthenticationBuilder.cs b/src/Microsoft.AspNetCore.Authentication/AuthenticationBuilder.cs index 54b4818851..3bce55ea10 100644 --- a/src/Microsoft.AspNetCore.Authentication/AuthenticationBuilder.cs +++ b/src/Microsoft.AspNetCore.Authentication/AuthenticationBuilder.cs @@ -97,10 +97,6 @@ namespace Microsoft.AspNetCore.Authentication public void PostConfigure(string name, TOptions options) { 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/AuthenticationHandler.cs b/src/Microsoft.AspNetCore.Authentication/AuthenticationHandler.cs index 812ba2f1a8..9728e5ff05 100644 --- a/src/Microsoft.AspNetCore.Authentication/AuthenticationHandler.cs +++ b/src/Microsoft.AspNetCore.Authentication/AuthenticationHandler.cs @@ -87,7 +87,7 @@ namespace Microsoft.AspNetCore.Authentication Context = context; Options = OptionsMonitor.Get(Scheme.Name) ?? new TOptions(); - Options.Validate(); + Options.Validate(Scheme.Name); await InitializeEventsAsync(); await InitializeHandlerAsync(); diff --git a/src/Microsoft.AspNetCore.Authentication/AuthenticationSchemeOptions.cs b/src/Microsoft.AspNetCore.Authentication/AuthenticationSchemeOptions.cs index 0e86b3a9ff..18d4c97881 100644 --- a/src/Microsoft.AspNetCore.Authentication/AuthenticationSchemeOptions.cs +++ b/src/Microsoft.AspNetCore.Authentication/AuthenticationSchemeOptions.cs @@ -13,9 +13,14 @@ namespace Microsoft.AspNetCore.Authentication /// /// Check that the options are valid. Should throw an exception if things are not ok. /// - public virtual void Validate() - { - } + public virtual void Validate() { } + + /// + /// Checks that the options are valid for a specific scheme + /// + /// The scheme being validated. + public virtual void Validate(string scheme) + => Validate(); /// /// Gets or sets the issuer that should be used for any claims that are created diff --git a/src/Microsoft.AspNetCore.Authentication/RemoteAuthenticationOptions.cs b/src/Microsoft.AspNetCore.Authentication/RemoteAuthenticationOptions.cs index 3b34cf43e9..daba1890fb 100644 --- a/src/Microsoft.AspNetCore.Authentication/RemoteAuthenticationOptions.cs +++ b/src/Microsoft.AspNetCore.Authentication/RemoteAuthenticationOptions.cs @@ -32,6 +32,19 @@ namespace Microsoft.AspNetCore.Authentication }; } + /// + /// Checks that the options are valid for a specific scheme + /// + /// The scheme being validated. + public override void Validate(string scheme) + { + base.Validate(scheme); + if (string.Equals(scheme, SignInScheme, StringComparison.Ordinal)) + { + throw new InvalidOperationException(Resources.Exception_RemoteSignInSchemeCannotBeSelf); + } + } + /// /// Check that the options are valid. Should throw an exception if things are not ok. /// diff --git a/test/Microsoft.AspNetCore.Authentication.Test/FacebookTests.cs b/test/Microsoft.AspNetCore.Authentication.Test/FacebookTests.cs index 81373403bd..2314b6b3c9 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/FacebookTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/FacebookTests.cs @@ -29,7 +29,11 @@ namespace Microsoft.AspNetCore.Authentication.Facebook { var server = CreateServer( app => { }, - services => services.AddAuthentication().AddFacebook(o => o.SignInScheme = FacebookDefaults.AuthenticationScheme), + services => services.AddAuthentication().AddFacebook(o => { + o.AppId = "whatever"; + o.AppSecret = "whatever"; + o.SignInScheme = FacebookDefaults.AuthenticationScheme; + }), context => { // Gross @@ -45,7 +49,10 @@ namespace Microsoft.AspNetCore.Authentication.Facebook { var server = CreateServer( app => { }, - services => services.AddAuthentication(o => o.DefaultScheme = FacebookDefaults.AuthenticationScheme).AddFacebook(), + services => services.AddAuthentication(o => o.DefaultScheme = FacebookDefaults.AuthenticationScheme).AddFacebook(o => { + o.AppId = "whatever"; + o.AppSecret = "whatever"; + }), context => { // Gross @@ -61,7 +68,10 @@ namespace Microsoft.AspNetCore.Authentication.Facebook { var server = CreateServer( app => { }, - services => services.AddAuthentication(o => o.DefaultSignInScheme = FacebookDefaults.AuthenticationScheme).AddFacebook(), + services => services.AddAuthentication(o => o.DefaultSignInScheme = FacebookDefaults.AuthenticationScheme).AddFacebook(o => { + o.AppId = "whatever"; + o.AppSecret = "whatever"; + }), context => { // Gross diff --git a/test/Microsoft.AspNetCore.Authentication.Test/OAuthTests.cs b/test/Microsoft.AspNetCore.Authentication.Test/OAuthTests.cs index 81d2360ec7..65d865b941 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/OAuthTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/OAuthTests.cs @@ -27,6 +27,9 @@ namespace Microsoft.AspNetCore.Authentication.OAuth o.SignInScheme = "weeblie"; o.ClientId = "whatever"; o.ClientSecret = "whatever"; + o.CallbackPath = "/whatever"; + o.AuthorizationEndpoint = "/whatever"; + o.TokenEndpoint = "/whatever"; })); var error = await Assert.ThrowsAsync(() => server.SendAsync("https://example.com/")); Assert.Contains("cannot be set to itself", error.Message);