From 06c93669d617fec50bf411c0c53d672019adf9df Mon Sep 17 00:00:00 2001 From: Patrick Westerhoff Date: Fri, 23 Mar 2018 02:09:05 +0100 Subject: [PATCH] Allow overwriting parameters on challenge requests Add a way to overwrite query arguments that are passed as part of the challenge request to the external authentication provider in OAuth-based authentication providers, including OpenID Connect. This uses the new `AuthenticationProperties.Parameters` collection to pass parameters to the authentication handler which will then look for special items within that property bag, overwriting previously configured values within the authentication options. This can be used for example to overwrite the OAuth scopes that are requested from an authentication provider, or to explicitly trigger a reauthentication by requiring a login prompt with OpenID Connect. By being able to specify this on individual challenge requests (using `HttpContext.ChallengeAsync`), this is independent from the global scheme configuration. Custom ~ChallengeProperties types, e.g. `OAuthChallengeProperties` for OAuth-based authentication providers, provide assistance in setting the challenge request parameters but are not required to make the handlers use the overwritten values. - Adjust authentication handlers to respect the custom parameters, and add ~ChallengeProperties types. - Introduce `OAuthHandler.FormatScope(IEnumerable)` to format a custom set of scopes. Subclasses requiring a different scope format should override this method instead of the parameterless overload. Overriding just `FormatScope()` will prevent handlers from supporting overwriting the OAuth `scope` in a challenge request. - Refactor GoogleHandler to support parameterization through both the `Parameters` and the `Items` collection (former is preferred) to keep compatibility with the old behavior. - Add an OpenIdConnect sample to overwrite the prompt argument in a challenge request. - Add extensive tests. --- samples/OpenIdConnectSample/Startup.cs | 17 ++ .../FacebookHandler.cs | 8 +- .../GoogleChallengeProperties.cs | 89 ++++++++++ .../GoogleHandler.cs | 61 ++++--- .../OAuthChallengeProperties.cs | 41 +++++ .../OAuthHandler.cs | 23 ++- .../OpenIdConnectChallengeProperties.cs | 49 ++++++ .../OpenIdConnectHandler.cs | 11 +- .../FacebookTests.cs | 91 ++++++++++ .../GoogleTests.cs | 138 ++++++++++++++- .../MicrosoftAccountTests.cs | 63 +++++++ .../OAuthChallengePropertiesTest.cs | 149 ++++++++++++++++ .../OAuthTests.cs | 82 +++++++++ .../OpenIdConnectChallengeTests.cs | 166 +++++++++++++++++- .../OpenIdConnect/TestSettings.cs | 6 + 15 files changed, 944 insertions(+), 50 deletions(-) create mode 100644 src/Microsoft.AspNetCore.Authentication.Google/GoogleChallengeProperties.cs create mode 100644 src/Microsoft.AspNetCore.Authentication.OAuth/OAuthChallengeProperties.cs create mode 100644 src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectChallengeProperties.cs create mode 100644 test/Microsoft.AspNetCore.Authentication.Test/OAuthChallengePropertiesTest.cs diff --git a/samples/OpenIdConnectSample/Startup.cs b/samples/OpenIdConnectSample/Startup.cs index 82bfdf54f1..1aa7625cb0 100644 --- a/samples/OpenIdConnectSample/Startup.cs +++ b/samples/OpenIdConnectSample/Startup.cs @@ -225,11 +225,28 @@ namespace OpenIdConnectSample return; } + if (context.Request.Path.Equals("/login-challenge")) + { + // Challenge the user authentication, and force a login prompt by overwriting the + // "prompt". This could be used for example to require the user to re-enter their + // credentials at the authentication provider, to add an extra confirmation layer. + await context.ChallengeAsync(OpenIdConnectDefaults.AuthenticationScheme, new OpenIdConnectChallengeProperties() + { + Prompt = "login", + + // it is also possible to specify different scopes, e.g. + // Scope = new string[] { "openid", "profile", "other" } + }); + + return; + } + await WriteHtmlAsync(response, async res => { await res.WriteAsync($"

Hello Authenticated User {HtmlEncode(user.Identity.Name)}

"); await res.WriteAsync("Refresh tokens"); await res.WriteAsync("Restricted"); + await res.WriteAsync("Login challenge"); await res.WriteAsync("Sign Out"); await res.WriteAsync("Sign Out Remote"); diff --git a/src/Microsoft.AspNetCore.Authentication.Facebook/FacebookHandler.cs b/src/Microsoft.AspNetCore.Authentication.Facebook/FacebookHandler.cs index 0f83c17196..eb42511431 100644 --- a/src/Microsoft.AspNetCore.Authentication.Facebook/FacebookHandler.cs +++ b/src/Microsoft.AspNetCore.Authentication.Facebook/FacebookHandler.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System.Collections.Generic; using System.Globalization; using System.Net.Http; using System.Security.Claims; @@ -65,12 +66,15 @@ namespace Microsoft.AspNetCore.Authentication.Facebook } } - protected override string FormatScope() + protected override string FormatScope(IEnumerable scopes) { // Facebook deviates from the OAuth spec here. They require comma separated instead of space separated. // https://developers.facebook.com/docs/reference/dialogs/oauth // http://tools.ietf.org/html/rfc6749#section-3.3 - return string.Join(",", Options.Scope); + return string.Join(",", scopes); } + + protected override string FormatScope() + => base.FormatScope(); } } diff --git a/src/Microsoft.AspNetCore.Authentication.Google/GoogleChallengeProperties.cs b/src/Microsoft.AspNetCore.Authentication.Google/GoogleChallengeProperties.cs new file mode 100644 index 0000000000..714df45655 --- /dev/null +++ b/src/Microsoft.AspNetCore.Authentication.Google/GoogleChallengeProperties.cs @@ -0,0 +1,89 @@ +using System.Collections.Generic; +using Microsoft.AspNetCore.Authentication.OAuth; + +namespace Microsoft.AspNetCore.Authentication.Google +{ + public class GoogleChallengeProperties : OAuthChallengeProperties + { + /// + /// The parameter key for the "access_type" argument being used for a challenge request. + /// + public static readonly string AccessTypeKey = "access_type"; + + /// + /// The parameter key for the "approval_prompt" argument being used for a challenge request. + /// + public static readonly string ApprovalPromptKey = "approval_prompt"; + + /// + /// The parameter key for the "include_granted_scopes" argument being used for a challenge request. + /// + public static readonly string IncludeGrantedScopesKey = "include_granted_scopes"; + + /// + /// The parameter key for the "login_hint" argument being used for a challenge request. + /// + public static readonly string LoginHintKey = "login_hint"; + + /// + /// The parameter key for the "prompt" argument being used for a challenge request. + /// + public static readonly string PromptParameterKey = "prompt"; + + public GoogleChallengeProperties() + { } + + public GoogleChallengeProperties(IDictionary items) + : base(items) + { } + + public GoogleChallengeProperties(IDictionary items, IDictionary parameters) + : base(items, parameters) + { } + + /// + /// The "access_type" parameter value being used for a challenge request. + /// + public string AccessType + { + get => GetParameter(AccessTypeKey); + set => SetParameter(AccessTypeKey, value); + } + + /// + /// The "approval_prompt" parameter value being used for a challenge request. + /// + public string ApprovalPrompt + { + get => GetParameter(ApprovalPromptKey); + set => SetParameter(ApprovalPromptKey, value); + } + + /// + /// The "include_granted_scopes" parameter value being used for a challenge request. + /// + public bool? IncludeGrantedScopes + { + get => GetParameter(IncludeGrantedScopesKey); + set => SetParameter(IncludeGrantedScopesKey, value); + } + + /// + /// The "login_hint" parameter value being used for a challenge request. + /// + public string LoginHint + { + get => GetParameter(LoginHintKey); + set => SetParameter(LoginHintKey, value); + } + + /// + /// The "prompt" parameter value being used for a challenge request. + /// + public string Prompt + { + get => GetParameter(PromptParameterKey); + set => SetParameter(PromptParameterKey, value); + } + } +} diff --git a/src/Microsoft.AspNetCore.Authentication.Google/GoogleHandler.cs b/src/Microsoft.AspNetCore.Authentication.Google/GoogleHandler.cs index 091896f7cf..88d48d4467 100644 --- a/src/Microsoft.AspNetCore.Authentication.Google/GoogleHandler.cs +++ b/src/Microsoft.AspNetCore.Authentication.Google/GoogleHandler.cs @@ -57,12 +57,12 @@ namespace Microsoft.AspNetCore.Authentication.Google queryStrings.Add("client_id", Options.ClientId); queryStrings.Add("redirect_uri", redirectUri); - AddQueryString(queryStrings, properties, "scope", FormatScope()); - AddQueryString(queryStrings, properties, "access_type", Options.AccessType); - AddQueryString(queryStrings, properties, "approval_prompt"); - AddQueryString(queryStrings, properties, "prompt"); - AddQueryString(queryStrings, properties, "login_hint"); - AddQueryString(queryStrings, properties, "include_granted_scopes"); + AddQueryString(queryStrings, properties, GoogleChallengeProperties.ScopeKey, FormatScope, Options.Scope); + AddQueryString(queryStrings, properties, GoogleChallengeProperties.AccessTypeKey, Options.AccessType); + AddQueryString(queryStrings, properties, GoogleChallengeProperties.ApprovalPromptKey); + AddQueryString(queryStrings, properties, GoogleChallengeProperties.PromptParameterKey); + AddQueryString(queryStrings, properties, GoogleChallengeProperties.LoginHintKey); + AddQueryString(queryStrings, properties, GoogleChallengeProperties.IncludeGrantedScopesKey, v => v?.ToString().ToLower(), (bool?)null); var state = Options.StateDataFormat.Protect(properties); queryStrings.Add("state", state); @@ -71,29 +71,38 @@ namespace Microsoft.AspNetCore.Authentication.Google return authorizationEndpoint; } - private static void AddQueryString( + private void AddQueryString( + IDictionary queryStrings, + AuthenticationProperties properties, + string name, + Func formatter, + T defaultValue) + { + string value = null; + var parameterValue = properties.GetParameter(name); + if (parameterValue != null) + { + value = formatter(parameterValue); + } + else if (!properties.Items.TryGetValue(name, out value)) + { + value = formatter(defaultValue); + } + + // Remove the parameter from AuthenticationProperties so it won't be serialized into the state + properties.Items.Remove(name); + + if (value != null) + { + queryStrings[name] = value; + } + } + + private void AddQueryString( IDictionary queryStrings, AuthenticationProperties properties, string name, string defaultValue = null) - { - string value; - if (!properties.Items.TryGetValue(name, out value)) - { - value = defaultValue; - } - else - { - // Remove the parameter from AuthenticationProperties so it won't be serialized to state parameter - properties.Items.Remove(name); - } - - if (value == null) - { - return; - } - - queryStrings[name] = value; - } + => AddQueryString(queryStrings, properties, name, x => x, defaultValue); } } diff --git a/src/Microsoft.AspNetCore.Authentication.OAuth/OAuthChallengeProperties.cs b/src/Microsoft.AspNetCore.Authentication.OAuth/OAuthChallengeProperties.cs new file mode 100644 index 0000000000..fc768a8ac8 --- /dev/null +++ b/src/Microsoft.AspNetCore.Authentication.OAuth/OAuthChallengeProperties.cs @@ -0,0 +1,41 @@ +using System.Collections.Generic; + +namespace Microsoft.AspNetCore.Authentication.OAuth +{ + public class OAuthChallengeProperties : AuthenticationProperties + { + /// + /// The parameter key for the "scope" argument being used for a challenge request. + /// + public static readonly string ScopeKey = "scope"; + + public OAuthChallengeProperties() + { } + + public OAuthChallengeProperties(IDictionary items) + : base(items) + { } + + public OAuthChallengeProperties(IDictionary items, IDictionary parameters) + : base(items, parameters) + { } + + /// + /// The "scope" parameter value being used for a challenge request. + /// + public ICollection Scope + { + get => GetParameter>(ScopeKey); + set => SetParameter(ScopeKey, value); + } + + /// + /// Set the "scope" parameter value. + /// + /// List of scopes. + public virtual void SetScope(params string[] scopes) + { + Scope = scopes; + } + } +} diff --git a/src/Microsoft.AspNetCore.Authentication.OAuth/OAuthHandler.cs b/src/Microsoft.AspNetCore.Authentication.OAuth/OAuthHandler.cs index 80680a7cf8..808e0f9039 100644 --- a/src/Microsoft.AspNetCore.Authentication.OAuth/OAuthHandler.cs +++ b/src/Microsoft.AspNetCore.Authentication.OAuth/OAuthHandler.cs @@ -209,7 +209,8 @@ namespace Microsoft.AspNetCore.Authentication.OAuth protected virtual string BuildChallengeUrl(AuthenticationProperties properties, string redirectUri) { - var scope = FormatScope(); + var scopeParameter = properties.GetParameter>(OAuthChallengeProperties.ScopeKey); + var scope = scopeParameter != null ? FormatScope(scopeParameter) : FormatScope(); var state = Options.StateDataFormat.Protect(properties); var parameters = new Dictionary @@ -223,10 +224,20 @@ namespace Microsoft.AspNetCore.Authentication.OAuth return QueryHelpers.AddQueryString(Options.AuthorizationEndpoint, parameters); } + /// + /// Format a list of OAuth scopes. + /// + /// List of scopes. + /// Formatted scopes. + protected virtual string FormatScope(IEnumerable scopes) + => string.Join(" ", scopes); // OAuth2 3.3 space separated + + /// + /// Format the property. + /// + /// Formatted scopes. + /// Subclasses should rather override . protected virtual string FormatScope() - { - // OAuth2 3.3 space separated - return string.Join(" ", Options.Scope); - } + => FormatScope(Options.Scope); } -} \ No newline at end of file +} diff --git a/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectChallengeProperties.cs b/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectChallengeProperties.cs new file mode 100644 index 0000000000..0ced488deb --- /dev/null +++ b/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectChallengeProperties.cs @@ -0,0 +1,49 @@ +using System; +using System.Collections.Generic; +using Microsoft.AspNetCore.Authentication.OAuth; +using Microsoft.IdentityModel.Protocols.OpenIdConnect; + +namespace Microsoft.AspNetCore.Authentication.OpenIdConnect +{ + public class OpenIdConnectChallengeProperties : OAuthChallengeProperties + { + /// + /// The parameter key for the "max_age" argument being used for a challenge request. + /// + public static readonly string MaxAgeKey = OpenIdConnectParameterNames.MaxAge; + + /// + /// The parameter key for the "prompt" argument being used for a challenge request. + /// + public static readonly string PromptKey = OpenIdConnectParameterNames.Prompt; + + public OpenIdConnectChallengeProperties() + { } + + public OpenIdConnectChallengeProperties(IDictionary items) + : base(items) + { } + + public OpenIdConnectChallengeProperties(IDictionary items, IDictionary parameters) + : base(items, parameters) + { } + + /// + /// The "max_age" parameter value being used for a challenge request. + /// + public TimeSpan? MaxAge + { + get => GetParameter(MaxAgeKey); + set => SetParameter(MaxAgeKey, value); + } + + /// + /// The "prompt" parameter value being used for a challenge request. + /// + public string Prompt + { + get => GetParameter(PromptKey); + set => SetParameter(PromptKey, value); + } + } +} diff --git a/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectHandler.cs b/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectHandler.cs index ce7494fb4a..029cf541b7 100644 --- a/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectHandler.cs +++ b/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectHandler.cs @@ -329,15 +329,16 @@ namespace Microsoft.AspNetCore.Authentication.OpenIdConnect RedirectUri = BuildRedirectUri(Options.CallbackPath), Resource = Options.Resource, ResponseType = Options.ResponseType, - Prompt = Options.Prompt, - Scope = string.Join(" ", Options.Scope) + Prompt = properties.GetParameter(OpenIdConnectParameterNames.Prompt) ?? Options.Prompt, + Scope = string.Join(" ", properties.GetParameter>(OpenIdConnectParameterNames.Scope) ?? Options.Scope), }; // Add the 'max_age' parameter to the authentication request if MaxAge is not null. // See http://openid.net/specs/openid-connect-core-1_0.html#AuthRequest - if (Options.MaxAge.HasValue) + var maxAge = properties.GetParameter(OpenIdConnectParameterNames.MaxAge) ?? Options.MaxAge; + if (maxAge.HasValue) { - message.MaxAge = Convert.ToInt64(Math.Floor((Options.MaxAge.Value).TotalSeconds)) + message.MaxAge = Convert.ToInt64(Math.Floor((maxAge.Value).TotalSeconds)) .ToString(CultureInfo.InvariantCulture); } @@ -783,7 +784,7 @@ namespace Microsoft.AspNetCore.Authentication.OpenIdConnect /// The authentication properties. /// which is used to determine if the remote authentication was successful. protected virtual async Task GetUserInformationAsync( - OpenIdConnectMessage message, JwtSecurityToken jwt, + OpenIdConnectMessage message, JwtSecurityToken jwt, ClaimsPrincipal principal, AuthenticationProperties properties) { var userInfoEndpoint = _configuration?.UserInfoEndpoint; diff --git a/test/Microsoft.AspNetCore.Authentication.Test/FacebookTests.cs b/test/Microsoft.AspNetCore.Authentication.Test/FacebookTests.cs index 4ee9117f95..b909be9fdc 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/FacebookTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/FacebookTests.cs @@ -559,6 +559,97 @@ namespace Microsoft.AspNetCore.Authentication.Facebook Assert.Contains("custom=test", query); } + [Fact] + public async Task ChallengeWillIncludeScopeAsConfigured() + { + var server = CreateServer( + app => app.UseAuthentication(), + services => + { + services.AddAuthentication().AddFacebook(o => + { + o.AppId = "Test App Id"; + o.AppSecret = "Test App Secret"; + o.Scope.Clear(); + o.Scope.Add("foo"); + o.Scope.Add("bar"); + }); + }, + async context => + { + await context.ChallengeAsync(FacebookDefaults.AuthenticationScheme); + return true; + }); + + var transaction = await server.SendAsync("http://example.com/challenge"); + var res = transaction.Response; + + Assert.Equal(HttpStatusCode.Redirect, res.StatusCode); + Assert.Contains("scope=foo,bar", res.Headers.Location.Query); + } + + [Fact] + public async Task ChallengeWillIncludeScopeAsOverwritten() + { + var server = CreateServer( + app => app.UseAuthentication(), + services => + { + services.AddAuthentication().AddFacebook(o => + { + o.AppId = "Test App Id"; + o.AppSecret = "Test App Secret"; + o.Scope.Clear(); + o.Scope.Add("foo"); + o.Scope.Add("bar"); + }); + }, + async context => + { + var properties = new OAuthChallengeProperties(); + properties.SetScope("baz", "qux"); + await context.ChallengeAsync(FacebookDefaults.AuthenticationScheme, properties); + return true; + }); + + var transaction = await server.SendAsync("http://example.com/challenge"); + var res = transaction.Response; + + Assert.Equal(HttpStatusCode.Redirect, res.StatusCode); + Assert.Contains("scope=baz,qux", res.Headers.Location.Query); + } + + [Fact] + public async Task ChallengeWillIncludeScopeAsOverwrittenWithBaseAuthenticationProperties() + { + var server = CreateServer( + app => app.UseAuthentication(), + services => + { + services.AddAuthentication().AddFacebook(o => + { + o.AppId = "Test App Id"; + o.AppSecret = "Test App Secret"; + o.Scope.Clear(); + o.Scope.Add("foo"); + o.Scope.Add("bar"); + }); + }, + async context => + { + var properties = new AuthenticationProperties(); + properties.SetParameter(OAuthChallengeProperties.ScopeKey, new string[] { "baz", "qux" }); + await context.ChallengeAsync(FacebookDefaults.AuthenticationScheme, properties); + return true; + }); + + var transaction = await server.SendAsync("http://example.com/challenge"); + var res = transaction.Response; + + Assert.Equal(HttpStatusCode.Redirect, res.StatusCode); + Assert.Contains("scope=baz,qux", res.Headers.Location.Query); + } + [Fact] public async Task NestedMapWillNotAffectRedirect() { diff --git a/test/Microsoft.AspNetCore.Authentication.Test/GoogleTests.cs b/test/Microsoft.AspNetCore.Authentication.Test/GoogleTests.cs index d9af959360..511a658ff4 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/GoogleTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/GoogleTests.cs @@ -16,6 +16,7 @@ using Microsoft.AspNetCore.DataProtection; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.TestHost; +using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; using Newtonsoft.Json; @@ -545,12 +546,65 @@ namespace Microsoft.AspNetCore.Authentication.Google } [Fact] - public async Task ChallengeWillUseAuthenticationPropertiesAsParameters() + public async Task ChallengeWillUseAuthenticationPropertiesParametersAsQueryArguments() { + var stateFormat = new PropertiesDataFormat(new EphemeralDataProtectionProvider(NullLoggerFactory.Instance).CreateProtector("GoogleTest")); var server = CreateServer(o => { o.ClientId = "Test Id"; o.ClientSecret = "Test Secret"; + o.StateDataFormat = stateFormat; + }, + context => + { + var req = context.Request; + var res = context.Response; + if (req.Path == new PathString("/challenge2")) + { + return context.ChallengeAsync("Google", new GoogleChallengeProperties + { + Scope = new string[] { "openid", "https://www.googleapis.com/auth/plus.login" }, + AccessType = "offline", + ApprovalPrompt = "force", + Prompt = "consent", + LoginHint = "test@example.com", + IncludeGrantedScopes = false, + }); + } + + return Task.FromResult(null); + }); + var transaction = await server.SendAsync("https://example.com/challenge2"); + Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode); + + // verify query arguments + var query = QueryHelpers.ParseQuery(transaction.Response.Headers.Location.Query); + Assert.Equal("openid https://www.googleapis.com/auth/plus.login", query["scope"]); + Assert.Equal("offline", query["access_type"]); + Assert.Equal("force", query["approval_prompt"]); + Assert.Equal("consent", query["prompt"]); + Assert.Equal("false", query["include_granted_scopes"]); + Assert.Equal("test@example.com", query["login_hint"]); + + // verify that the passed items were not serialized + var stateProperties = stateFormat.Unprotect(query["state"]); + Assert.DoesNotContain("scope", stateProperties.Items.Keys); + Assert.DoesNotContain("access_type", stateProperties.Items.Keys); + Assert.DoesNotContain("include_granted_scopes", stateProperties.Items.Keys); + Assert.DoesNotContain("approval_prompt", stateProperties.Items.Keys); + Assert.DoesNotContain("prompt", stateProperties.Items.Keys); + Assert.DoesNotContain("login_hint", stateProperties.Items.Keys); + } + + [Fact] + public async Task ChallengeWillUseAuthenticationPropertiesItemsAsParameters() + { + var stateFormat = new PropertiesDataFormat(new EphemeralDataProtectionProvider(NullLoggerFactory.Instance).CreateProtector("GoogleTest")); + var server = CreateServer(o => + { + o.ClientId = "Test Id"; + o.ClientSecret = "Test Secret"; + o.StateDataFormat = stateFormat; }, context => { @@ -573,13 +627,79 @@ namespace Microsoft.AspNetCore.Authentication.Google }); var transaction = await server.SendAsync("https://example.com/challenge2"); Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode); - var query = transaction.Response.Headers.Location.Query; - Assert.Contains("scope=" + UrlEncoder.Default.Encode("https://www.googleapis.com/auth/plus.login"), query); - Assert.Contains("access_type=offline", query); - Assert.Contains("approval_prompt=force", query); - Assert.Contains("prompt=consent", query); - Assert.Contains("include_granted_scopes=false", query); - Assert.Contains("login_hint=" + UrlEncoder.Default.Encode("test@example.com"), query); + + // verify query arguments + var query = QueryHelpers.ParseQuery(transaction.Response.Headers.Location.Query); + Assert.Equal("https://www.googleapis.com/auth/plus.login", query["scope"]); + Assert.Equal("offline", query["access_type"]); + Assert.Equal("force", query["approval_prompt"]); + Assert.Equal("consent", query["prompt"]); + Assert.Equal("false", query["include_granted_scopes"]); + Assert.Equal("test@example.com", query["login_hint"]); + + // verify that the passed items were not serialized + var stateProperties = stateFormat.Unprotect(query["state"]); + Assert.DoesNotContain("scope", stateProperties.Items.Keys); + Assert.DoesNotContain("access_type", stateProperties.Items.Keys); + Assert.DoesNotContain("include_granted_scopes", stateProperties.Items.Keys); + Assert.DoesNotContain("approval_prompt", stateProperties.Items.Keys); + Assert.DoesNotContain("prompt", stateProperties.Items.Keys); + Assert.DoesNotContain("login_hint", stateProperties.Items.Keys); + } + + [Fact] + public async Task ChallengeWillUseAuthenticationPropertiesItemsAsQueryArgumentsButParametersWillOverwrite() + { + var stateFormat = new PropertiesDataFormat(new EphemeralDataProtectionProvider(NullLoggerFactory.Instance).CreateProtector("GoogleTest")); + var server = CreateServer(o => + { + o.ClientId = "Test Id"; + o.ClientSecret = "Test Secret"; + o.StateDataFormat = stateFormat; + }, + context => + { + var req = context.Request; + var res = context.Response; + if (req.Path == new PathString("/challenge2")) + { + return context.ChallengeAsync("Google", new GoogleChallengeProperties(new Dictionary + { + ["scope"] = "https://www.googleapis.com/auth/plus.login", + ["access_type"] = "offline", + ["include_granted_scopes"] = "false", + ["approval_prompt"] = "force", + ["prompt"] = "login", + ["login_hint"] = "this-will-be-overwritten@example.com", + }) + { + Prompt = "consent", + LoginHint = "test@example.com", + }); + } + + return Task.FromResult(null); + }); + var transaction = await server.SendAsync("https://example.com/challenge2"); + Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode); + + // verify query arguments + var query = QueryHelpers.ParseQuery(transaction.Response.Headers.Location.Query); + Assert.Equal("https://www.googleapis.com/auth/plus.login", query["scope"]); + Assert.Equal("offline", query["access_type"]); + Assert.Equal("force", query["approval_prompt"]); + Assert.Equal("consent", query["prompt"]); + Assert.Equal("false", query["include_granted_scopes"]); + Assert.Equal("test@example.com", query["login_hint"]); + + // verify that the passed items were not serialized + var stateProperties = stateFormat.Unprotect(query["state"]); + Assert.DoesNotContain("scope", stateProperties.Items.Keys); + Assert.DoesNotContain("access_type", stateProperties.Items.Keys); + Assert.DoesNotContain("include_granted_scopes", stateProperties.Items.Keys); + Assert.DoesNotContain("approval_prompt", stateProperties.Items.Keys); + Assert.DoesNotContain("prompt", stateProperties.Items.Keys); + Assert.DoesNotContain("login_hint", stateProperties.Items.Keys); } [Fact] @@ -1499,4 +1619,4 @@ namespace Microsoft.AspNetCore.Authentication.Google } } } -} \ No newline at end of file +} diff --git a/test/Microsoft.AspNetCore.Authentication.Test/MicrosoftAccountTests.cs b/test/Microsoft.AspNetCore.Authentication.Test/MicrosoftAccountTests.cs index 480241d35b..e2e13f270e 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/MicrosoftAccountTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/MicrosoftAccountTests.cs @@ -525,6 +525,57 @@ namespace Microsoft.AspNetCore.Authentication.Tests.MicrosoftAccount Assert.Contains("state=", location); } + [Fact] + public async Task ChallengeWillIncludeScopeAsConfigured() + { + var server = CreateServer(o => + { + o.ClientId = "Test Id"; + o.ClientSecret = "Test Secret"; + o.Scope.Clear(); + o.Scope.Add("foo"); + o.Scope.Add("bar"); + }); + var transaction = await server.SendAsync("http://example.com/challenge"); + var res = transaction.Response; + Assert.Equal(HttpStatusCode.Redirect, res.StatusCode); + Assert.Contains("scope=foo%20bar", res.Headers.Location.Query); + } + + [Fact] + public async Task ChallengeWillIncludeScopeAsOverwritten() + { + var server = CreateServer(o => + { + o.ClientId = "Test Id"; + o.ClientSecret = "Test Secret"; + o.Scope.Clear(); + o.Scope.Add("foo"); + o.Scope.Add("bar"); + }); + var transaction = await server.SendAsync("http://example.com/challengeWithOtherScope"); + var res = transaction.Response; + Assert.Equal(HttpStatusCode.Redirect, res.StatusCode); + Assert.Contains("scope=baz%20qux", res.Headers.Location.Query); + } + + [Fact] + public async Task ChallengeWillIncludeScopeAsOverwrittenWithBaseAuthenticationProperties() + { + var server = CreateServer(o => + { + o.ClientId = "Test Id"; + o.ClientSecret = "Test Secret"; + o.Scope.Clear(); + o.Scope.Add("foo"); + o.Scope.Add("bar"); + }); + var transaction = await server.SendAsync("http://example.com/challengeWithOtherScopeWithBaseAuthenticationProperties"); + var res = transaction.Response; + Assert.Equal(HttpStatusCode.Redirect, res.StatusCode); + Assert.Contains("scope=baz%20qux", res.Headers.Location.Query); + } + [Fact] public async Task AuthenticatedEventCanGetRefreshToken() { @@ -608,6 +659,18 @@ namespace Microsoft.AspNetCore.Authentication.Tests.MicrosoftAccount { await context.ChallengeAsync("Microsoft"); } + else if (req.Path == new PathString("/challengeWithOtherScope")) + { + var properties = new OAuthChallengeProperties(); + properties.SetScope("baz", "qux"); + await context.ChallengeAsync("Microsoft", properties); + } + else if (req.Path == new PathString("/challengeWithOtherScopeWithBaseAuthenticationProperties")) + { + var properties = new AuthenticationProperties(); + properties.SetParameter(OAuthChallengeProperties.ScopeKey, new string[] { "baz", "qux" }); + await context.ChallengeAsync("Microsoft", properties); + } else if (req.Path == new PathString("/me")) { res.Describe(context.User); diff --git a/test/Microsoft.AspNetCore.Authentication.Test/OAuthChallengePropertiesTest.cs b/test/Microsoft.AspNetCore.Authentication.Test/OAuthChallengePropertiesTest.cs new file mode 100644 index 0000000000..c359bb0e8c --- /dev/null +++ b/test/Microsoft.AspNetCore.Authentication.Test/OAuthChallengePropertiesTest.cs @@ -0,0 +1,149 @@ +using System; +using Microsoft.AspNetCore.Authentication.Google; +using Microsoft.AspNetCore.Authentication.OAuth; +using Microsoft.AspNetCore.Authentication.OpenIdConnect; +using Xunit; + +namespace Microsoft.AspNetCore.Authentication.Test +{ + public class OAuthChallengePropertiesTest + { + [Fact] + public void ScopeProperty() + { + var properties = new OAuthChallengeProperties + { + Scope = new string[] { "foo", "bar" } + }; + Assert.Equal(new string[] { "foo", "bar" }, properties.Scope); + Assert.Equal(new string[] { "foo", "bar" }, properties.Parameters["scope"]); + } + + [Fact] + public void ScopeProperty_NullValue() + { + var properties = new OAuthChallengeProperties(); + properties.Parameters["scope"] = new string[] { "foo", "bar" }; + Assert.Equal(new string[] { "foo", "bar" }, properties.Scope); + + properties.Scope = null; + Assert.Null(properties.Scope); + } + + [Fact] + public void SetScope() + { + var properties = new OAuthChallengeProperties(); + properties.SetScope("foo", "bar"); + Assert.Equal(new string[] { "foo", "bar" }, properties.Scope); + Assert.Equal(new string[] { "foo", "bar" }, properties.Parameters["scope"]); + } + + [Fact] + public void OidcMaxAge() + { + var properties = new OpenIdConnectChallengeProperties() + { + MaxAge = TimeSpan.FromSeconds(200) + }; + Assert.Equal(TimeSpan.FromSeconds(200), properties.MaxAge); + } + + [Fact] + public void OidcMaxAge_NullValue() + { + var properties = new OpenIdConnectChallengeProperties(); + properties.Parameters["max_age"] = TimeSpan.FromSeconds(500); + Assert.Equal(TimeSpan.FromSeconds(500), properties.MaxAge); + + properties.MaxAge = null; + Assert.Null(properties.MaxAge); + } + + [Fact] + public void OidcPrompt() + { + var properties = new OpenIdConnectChallengeProperties() + { + Prompt = "login" + }; + Assert.Equal("login", properties.Prompt); + Assert.Equal("login", properties.Parameters["prompt"]); + } + + [Fact] + public void OidcPrompt_NullValue() + { + var properties = new OpenIdConnectChallengeProperties(); + properties.Parameters["prompt"] = "consent"; + Assert.Equal("consent", properties.Prompt); + + properties.Prompt = null; + Assert.Null(properties.Prompt); + } + + [Fact] + public void GoogleProperties() + { + var properties = new GoogleChallengeProperties() + { + AccessType = "offline", + ApprovalPrompt = "force", + LoginHint = "test@example.com", + Prompt = "login", + }; + Assert.Equal("offline", properties.AccessType); + Assert.Equal("offline", properties.Parameters["access_type"]); + Assert.Equal("force", properties.ApprovalPrompt); + Assert.Equal("force", properties.Parameters["approval_prompt"]); + Assert.Equal("test@example.com", properties.LoginHint); + Assert.Equal("test@example.com", properties.Parameters["login_hint"]); + Assert.Equal("login", properties.Prompt); + Assert.Equal("login", properties.Parameters["prompt"]); + } + + [Fact] + public void GoogleProperties_NullValues() + { + var properties = new GoogleChallengeProperties(); + properties.Parameters["access_type"] = "offline"; + properties.Parameters["approval_prompt"] = "force"; + properties.Parameters["login_hint"] = "test@example.com"; + properties.Parameters["prompt"] = "login"; + Assert.Equal("offline", properties.AccessType); + Assert.Equal("force", properties.ApprovalPrompt); + Assert.Equal("test@example.com", properties.LoginHint); + Assert.Equal("login", properties.Prompt); + + properties.AccessType = null; + Assert.Null(properties.AccessType); + + properties.ApprovalPrompt = null; + Assert.Null(properties.ApprovalPrompt); + + properties.LoginHint = null; + Assert.Null(properties.LoginHint); + + properties.Prompt = null; + Assert.Null(properties.Prompt); + } + + [Fact] + public void GoogleIncludeGrantedScopes() + { + var properties = new GoogleChallengeProperties() + { + IncludeGrantedScopes = true + }; + Assert.True(properties.IncludeGrantedScopes); + Assert.Equal(true, properties.Parameters["include_granted_scopes"]); + + properties.IncludeGrantedScopes = false; + Assert.False(properties.IncludeGrantedScopes); + Assert.Equal(false, properties.Parameters["include_granted_scopes"]); + + properties.IncludeGrantedScopes = null; + Assert.Null(properties.IncludeGrantedScopes); + } + } +} diff --git a/test/Microsoft.AspNetCore.Authentication.Test/OAuthTests.cs b/test/Microsoft.AspNetCore.Authentication.Test/OAuthTests.cs index 9279f145b9..4b822b611f 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/OAuthTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/OAuthTests.cs @@ -572,6 +572,88 @@ namespace Microsoft.AspNetCore.Authentication.OAuth Assert.Contains("path=/", correlation); } + [Fact] + public async Task RedirectToAuthorizeEndpoint_HasScopeAsConfigured() + { + var server = CreateServer( + s => s.AddAuthentication().AddOAuth( + "Weblie", + opt => + { + ConfigureDefaults(opt); + opt.Scope.Clear(); + opt.Scope.Add("foo"); + opt.Scope.Add("bar"); + }), + async ctx => + { + await ctx.ChallengeAsync("Weblie"); + return true; + }); + + var transaction = await server.SendAsync("https://www.example.com/challenge"); + var res = transaction.Response; + + Assert.Equal(HttpStatusCode.Redirect, res.StatusCode); + Assert.Contains("scope=foo%20bar", res.Headers.Location.Query); + } + + [Fact] + public async Task RedirectToAuthorizeEndpoint_HasScopeAsOverwritten() + { + var server = CreateServer( + s => s.AddAuthentication().AddOAuth( + "Weblie", + opt => + { + ConfigureDefaults(opt); + opt.Scope.Clear(); + opt.Scope.Add("foo"); + opt.Scope.Add("bar"); + }), + async ctx => + { + var properties = new OAuthChallengeProperties(); + properties.SetScope("baz", "qux"); + await ctx.ChallengeAsync("Weblie", properties); + return true; + }); + + var transaction = await server.SendAsync("https://www.example.com/challenge"); + var res = transaction.Response; + + Assert.Equal(HttpStatusCode.Redirect, res.StatusCode); + Assert.Contains("scope=baz%20qux", res.Headers.Location.Query); + } + + [Fact] + public async Task RedirectToAuthorizeEndpoint_HasScopeAsOverwrittenWithBaseAuthenticationProperties() + { + var server = CreateServer( + s => s.AddAuthentication().AddOAuth( + "Weblie", + opt => + { + ConfigureDefaults(opt); + opt.Scope.Clear(); + opt.Scope.Add("foo"); + opt.Scope.Add("bar"); + }), + async ctx => + { + var properties = new AuthenticationProperties(); + properties.SetParameter(OAuthChallengeProperties.ScopeKey, new string[] { "baz", "qux" }); + await ctx.ChallengeAsync("Weblie", properties); + return true; + }); + + var transaction = await server.SendAsync("https://www.example.com/challenge"); + var res = transaction.Response; + + Assert.Equal(HttpStatusCode.Redirect, res.StatusCode); + Assert.Contains("scope=baz%20qux", res.Headers.Location.Query); + } + private void ConfigureDefaults(OAuthOptions o) { o.ClientId = "Test Id"; diff --git a/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectChallengeTests.cs b/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectChallengeTests.cs index 7ab81c9dd4..cbafc46223 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectChallengeTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectChallengeTests.cs @@ -51,14 +51,14 @@ namespace Microsoft.AspNetCore.Authentication.Test.OpenIdConnect [Fact] public async Task AuthorizationRequestDoesNotIncludeTelemetryParametersWhenDisabled() { - var setting = new TestSettings(opt => + var settings = new TestSettings(opt => { opt.ClientId = "Test Id"; opt.Authority = TestServerBuilder.DefaultAuthority; opt.DisableTelemetry = true; }); - var server = setting.CreateTestServer(); + var server = settings.CreateTestServer(); var transaction = await server.SendAsync(ChallengeEndpoint); var res = transaction.Response; @@ -425,6 +425,7 @@ namespace Microsoft.AspNetCore.Authentication.Test.OpenIdConnect var res = transaction.Response; + Assert.Equal(HttpStatusCode.Redirect, res.StatusCode); settings.ValidateChallengeRedirect( res.Headers.Location, OpenIdConnectParameterNames.MaxAge); @@ -446,9 +447,170 @@ namespace Microsoft.AspNetCore.Authentication.Test.OpenIdConnect var res = transaction.Response; + Assert.Equal(HttpStatusCode.Redirect, res.StatusCode); settings.ValidateChallengeRedirect( res.Headers.Location, OpenIdConnectParameterNames.MaxAge); } + + [Fact] + public async Task Challenge_HasExpectedPromptParam() + { + var settings = new TestSettings(opt => + { + opt.ClientId = "Test Id"; + opt.Authority = TestServerBuilder.DefaultAuthority; + opt.Prompt = "consent"; + }); + + var server = settings.CreateTestServer(); + var transaction = await server.SendAsync(ChallengeEndpoint); + + var res = transaction.Response; + + Assert.Equal(HttpStatusCode.Redirect, res.StatusCode); + settings.ValidateChallengeRedirect(res.Headers.Location, OpenIdConnectParameterNames.Prompt); + Assert.Contains("prompt=consent", res.Headers.Location.Query); + } + + [Fact] + public async Task Challenge_HasOverwrittenPromptParam() + { + var settings = new TestSettings(opt => + { + opt.ClientId = "Test Id"; + opt.Authority = TestServerBuilder.DefaultAuthority; + opt.Prompt = "consent"; + }); + var properties = new OpenIdConnectChallengeProperties() + { + Prompt = "login", + }; + + var server = settings.CreateTestServer(properties); + var transaction = await server.SendAsync(TestServerBuilder.TestHost + TestServerBuilder.ChallengeWithProperties); + + var res = transaction.Response; + + Assert.Equal(HttpStatusCode.Redirect, res.StatusCode); + settings.ValidateChallengeRedirect(res.Headers.Location); + Assert.Contains("prompt=login", res.Headers.Location.Query); + } + + [Fact] + public async Task Challenge_HasOverwrittenPromptParamFromBaseAuthenticationProperties() + { + var settings = new TestSettings(opt => + { + opt.ClientId = "Test Id"; + opt.Authority = TestServerBuilder.DefaultAuthority; + opt.Prompt = "consent"; + }); + var properties = new AuthenticationProperties(); + properties.SetParameter(OpenIdConnectChallengeProperties.PromptKey, "login"); + + var server = settings.CreateTestServer(properties); + var transaction = await server.SendAsync(TestServerBuilder.TestHost + TestServerBuilder.ChallengeWithProperties); + + var res = transaction.Response; + + Assert.Equal(HttpStatusCode.Redirect, res.StatusCode); + settings.ValidateChallengeRedirect(res.Headers.Location); + Assert.Contains("prompt=login", res.Headers.Location.Query); + } + + [Fact] + public async Task Challenge_HasOverwrittenScopeParam() + { + var settings = new TestSettings(opt => + { + opt.ClientId = "Test Id"; + opt.Authority = TestServerBuilder.DefaultAuthority; + opt.Scope.Clear(); + opt.Scope.Add("foo"); + opt.Scope.Add("bar"); + }); + var properties = new OpenIdConnectChallengeProperties(); + properties.SetScope("baz", "qux"); + + var server = settings.CreateTestServer(properties); + var transaction = await server.SendAsync(TestServerBuilder.TestHost + TestServerBuilder.ChallengeWithProperties); + + var res = transaction.Response; + + Assert.Equal(HttpStatusCode.Redirect, res.StatusCode); + settings.ValidateChallengeRedirect(res.Headers.Location); + Assert.Contains("scope=baz%20qux", res.Headers.Location.Query); + } + + [Fact] + public async Task Challenge_HasOverwrittenScopeParamFromBaseAuthenticationProperties() + { + var settings = new TestSettings(opt => + { + opt.ClientId = "Test Id"; + opt.Authority = TestServerBuilder.DefaultAuthority; + opt.Scope.Clear(); + opt.Scope.Add("foo"); + opt.Scope.Add("bar"); + }); + var properties = new AuthenticationProperties(); + properties.SetParameter(OpenIdConnectChallengeProperties.ScopeKey, new string[] { "baz", "qux" }); + + var server = settings.CreateTestServer(properties); + var transaction = await server.SendAsync(TestServerBuilder.TestHost + TestServerBuilder.ChallengeWithProperties); + + var res = transaction.Response; + + Assert.Equal(HttpStatusCode.Redirect, res.StatusCode); + settings.ValidateChallengeRedirect(res.Headers.Location); + Assert.Contains("scope=baz%20qux", res.Headers.Location.Query); + } + + [Fact] + public async Task Challenge_HasOverwrittenMaxAgeParam() + { + var settings = new TestSettings(opt => + { + opt.ClientId = "Test Id"; + opt.Authority = TestServerBuilder.DefaultAuthority; + opt.MaxAge = TimeSpan.FromSeconds(500); + }); + var properties = new OpenIdConnectChallengeProperties() + { + MaxAge = TimeSpan.FromSeconds(1234), + }; + + var server = settings.CreateTestServer(properties); + var transaction = await server.SendAsync(TestServerBuilder.TestHost + TestServerBuilder.ChallengeWithProperties); + + var res = transaction.Response; + + Assert.Equal(HttpStatusCode.Redirect, res.StatusCode); + settings.ValidateChallengeRedirect(res.Headers.Location); + Assert.Contains("max_age=1234", res.Headers.Location.Query); + } + + [Fact] + public async Task Challenge_HasOverwrittenMaxAgeParaFromBaseAuthenticationPropertiesm() + { + var settings = new TestSettings(opt => + { + opt.ClientId = "Test Id"; + opt.Authority = TestServerBuilder.DefaultAuthority; + opt.MaxAge = TimeSpan.FromSeconds(500); + }); + var properties = new AuthenticationProperties(); + properties.SetParameter(OpenIdConnectChallengeProperties.MaxAgeKey, TimeSpan.FromSeconds(1234)); + + var server = settings.CreateTestServer(properties); + var transaction = await server.SendAsync(TestServerBuilder.TestHost + TestServerBuilder.ChallengeWithProperties); + + var res = transaction.Response; + + Assert.Equal(HttpStatusCode.Redirect, res.StatusCode); + settings.ValidateChallengeRedirect(res.Headers.Location); + Assert.Contains("max_age=1234", res.Headers.Location.Query); + } } } \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/TestSettings.cs b/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/TestSettings.cs index 6bb5445dc6..a1e0233f3a 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/TestSettings.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/TestSettings.cs @@ -206,6 +206,9 @@ namespace Microsoft.AspNetCore.Authentication.Test.OpenIdConnect case OpenIdConnectParameterNames.MaxAge: ValidateMaxAge(actualValues, errors, htmlEncoded); break; + case OpenIdConnectParameterNames.Prompt: + ValidatePrompt(actualValues, errors, htmlEncoded); + break; default: throw new InvalidOperationException($"Unknown parameter \"{paramToValidate}\"."); } @@ -284,6 +287,9 @@ namespace Microsoft.AspNetCore.Authentication.Test.OpenIdConnect } } + private void ValidatePrompt(IDictionary actualParams, ICollection errors, bool htmlEncoded) => + ValidateParameter(OpenIdConnectParameterNames.Prompt, _options.Prompt, actualParams, errors, htmlEncoded); + private void ValidateParameter( string parameterName, string expectedValue,