diff --git a/src/Security/Authentication/Facebook/src/FacebookOptions.cs b/src/Security/Authentication/Facebook/src/FacebookOptions.cs index 51c5157b6885..b54c33914673 100644 --- a/src/Security/Authentication/Facebook/src/FacebookOptions.cs +++ b/src/Security/Authentication/Facebook/src/FacebookOptions.cs @@ -23,6 +23,7 @@ public FacebookOptions() AuthorizationEndpoint = FacebookDefaults.AuthorizationEndpoint; TokenEndpoint = FacebookDefaults.TokenEndpoint; UserInformationEndpoint = FacebookDefaults.UserInformationEndpoint; + UsePkce = true; Scope.Add("email"); Fields.Add("name"); Fields.Add("email"); diff --git a/src/Security/Authentication/Google/src/GoogleHandler.cs b/src/Security/Authentication/Google/src/GoogleHandler.cs index beda9af2daf8..33c10bb30d62 100644 --- a/src/Security/Authentication/Google/src/GoogleHandler.cs +++ b/src/Security/Authentication/Google/src/GoogleHandler.cs @@ -11,6 +11,7 @@ using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; +using Microsoft.Extensions.Primitives; namespace Microsoft.AspNetCore.Authentication.Google; @@ -58,27 +59,24 @@ protected override string BuildChallengeUrl(AuthenticationProperties properties, // Google Identity Platform Manual: // https://developers.google.com/identity/protocols/OAuth2WebServer - var queryStrings = new Dictionary(StringComparer.OrdinalIgnoreCase); - queryStrings.Add("response_type", "code"); - queryStrings.Add("client_id", Options.ClientId); - queryStrings.Add("redirect_uri", redirectUri); + // Some query params and features (e.g. PKCE) are handled by the base class but some params have to be modified or added here + var queryStrings = QueryHelpers.ParseQuery(new Uri(base.BuildChallengeUrl(properties, redirectUri)).Query); - 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(CultureInfo.InvariantCulture).ToLowerInvariant(), (bool?)null); + SetQueryParam(queryStrings, properties, GoogleChallengeProperties.ScopeKey, FormatScope, Options.Scope); + SetQueryParam(queryStrings, properties, GoogleChallengeProperties.AccessTypeKey, Options.AccessType); + SetQueryParam(queryStrings, properties, GoogleChallengeProperties.ApprovalPromptKey); + SetQueryParam(queryStrings, properties, GoogleChallengeProperties.PromptParameterKey); + SetQueryParam(queryStrings, properties, GoogleChallengeProperties.LoginHintKey); + SetQueryParam(queryStrings, properties, GoogleChallengeProperties.IncludeGrantedScopesKey, v => v?.ToString(CultureInfo.InvariantCulture).ToLowerInvariant(), (bool?)null); - var state = Options.StateDataFormat.Protect(properties); - queryStrings.Add("state", state); + // Some properties are removed when setting query params above, so the state has to be reset + queryStrings["state"] = Options.StateDataFormat.Protect(properties); - var authorizationEndpoint = QueryHelpers.AddQueryString(Options.AuthorizationEndpoint, queryStrings!); - return authorizationEndpoint; + return QueryHelpers.AddQueryString(Options.AuthorizationEndpoint, queryStrings); } - private static void AddQueryString( - IDictionary queryStrings, + private static void SetQueryParam( + IDictionary queryStrings, AuthenticationProperties properties, string name, Func formatter, @@ -104,10 +102,10 @@ private static void AddQueryString( } } - private static void AddQueryString( - IDictionary queryStrings, + private static void SetQueryParam( + IDictionary queryStrings, AuthenticationProperties properties, string name, string? defaultValue = null) - => AddQueryString(queryStrings, properties, name, x => x, defaultValue); + => SetQueryParam(queryStrings, properties, name, x => x, defaultValue); } diff --git a/src/Security/Authentication/Google/src/GoogleOptions.cs b/src/Security/Authentication/Google/src/GoogleOptions.cs index 9408148ebc0d..1eb93c297454 100644 --- a/src/Security/Authentication/Google/src/GoogleOptions.cs +++ b/src/Security/Authentication/Google/src/GoogleOptions.cs @@ -21,6 +21,7 @@ public GoogleOptions() AuthorizationEndpoint = GoogleDefaults.AuthorizationEndpoint; TokenEndpoint = GoogleDefaults.TokenEndpoint; UserInformationEndpoint = GoogleDefaults.UserInformationEndpoint; + UsePkce = true; Scope.Add("openid"); Scope.Add("profile"); Scope.Add("email"); diff --git a/src/Security/Authentication/test/FacebookTests.cs b/src/Security/Authentication/test/FacebookTests.cs index 001fea35feea..61cd2de5cf8c 100644 --- a/src/Security/Authentication/test/FacebookTests.cs +++ b/src/Security/Authentication/test/FacebookTests.cs @@ -12,6 +12,7 @@ using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.TestHost; +using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging.Abstractions; @@ -303,6 +304,8 @@ public async Task ChallengeWillTriggerRedirection() Assert.Contains("redirect_uri=", location); Assert.Contains("scope=", location); Assert.Contains("state=", location); + Assert.Contains("code_challenge=", location); + Assert.Contains("code_challenge_method=S256", location); } [Fact] @@ -368,6 +371,87 @@ public async Task CustomUserInfoEndpointHasValidGraphQuery() Assert.Contains("&appsecret_proof=b7fb6d5a4510926b4af6fe080497827d791dc45fe6541d88ba77bdf6e8e208c6&", finalUserInfoEndpoint); } + [Fact] + public async Task PkceSentToTokenEndpoint() + { + using var host = await CreateHost( + app => app.UseAuthentication(), + services => + { + services.AddAuthentication(TestExtensions.CookieAuthenticationScheme) + .AddCookie(TestExtensions.CookieAuthenticationScheme) + .AddFacebook(o => + { + o.AppId = "Test App Id"; + o.AppSecret = "Test App Secret"; + o.BackchannelHttpHandler = new TestHttpMessageHandler + { + Sender = req => + { + if (req.RequestUri.AbsoluteUri == "https://graph.facebook.com/v14.0/oauth/access_token") + { + var body = req.Content.ReadAsStringAsync().Result; + var form = new FormReader(body); + var entries = form.ReadForm(); + Assert.Equal("Test App Id", entries["client_id"]); + Assert.Equal("https://example.com/signin-facebook", entries["redirect_uri"]); + Assert.Equal("Test App Secret", entries["client_secret"]); + Assert.Equal("TestCode", entries["code"]); + Assert.Equal("authorization_code", entries["grant_type"]); + Assert.False(string.IsNullOrEmpty(entries["code_verifier"])); + + return ReturnJsonResponse(new + { + access_token = "Test Access Token", + expire_in = 3600, + token_type = "Bearer", + }); + } + else if (req.RequestUri.GetComponents(UriComponents.SchemeAndServer | UriComponents.Path, UriFormat.UriEscaped) == "https://graph.facebook.com/v14.0/me") + { + return ReturnJsonResponse(new + { + id = "Test User ID", + displayName = "Test Name", + givenName = "Test Given Name", + surname = "Test Family Name", + mail = "Test email" + }); + } + + return null; + } + }; + }); + }, + async context => + { + await context.ChallengeAsync("Facebook"); + return true; + }); + using var server = host.GetTestServer(); + var transaction = await server.SendAsync("https://example.com/challenge"); + Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode); + var locationUri = transaction.Response.Headers.Location; + Assert.StartsWith("https://www.facebook.com/v14.0/dialog/oauth", locationUri.AbsoluteUri); + + var queryParams = QueryHelpers.ParseQuery(locationUri.Query); + Assert.False(string.IsNullOrEmpty(queryParams["code_challenge"])); + Assert.Equal("S256", queryParams["code_challenge_method"]); + + var nonceCookie = transaction.SetCookie.Single(); + nonceCookie = nonceCookie.Substring(0, nonceCookie.IndexOf(';')); + + transaction = await server.SendAsync( + "https://example.com/signin-facebook?code=TestCode&state=" + queryParams["state"], + nonceCookie); + Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode); + Assert.Equal("/challenge", transaction.Response.Headers.GetValues("Location").First()); + Assert.Equal(2, transaction.SetCookie.Count); + Assert.StartsWith(".AspNetCore.Correlation.", transaction.SetCookie[0]); + Assert.StartsWith(".AspNetCore." + TestExtensions.CookieAuthenticationScheme, transaction.SetCookie[1]); + } + private static async Task CreateHost(Action configure, Action configureServices, Func> handler) { var host = new HostBuilder() @@ -390,4 +474,12 @@ private static async Task CreateHost(Action configur await host.StartAsync(); return host; } + + private static HttpResponseMessage ReturnJsonResponse(object content, HttpStatusCode code = HttpStatusCode.OK) + { + var res = new HttpResponseMessage(code); + var text = Newtonsoft.Json.JsonConvert.SerializeObject(content); + res.Content = new StringContent(text, Encoding.UTF8, "application/json"); + return res; + } } diff --git a/src/Security/Authentication/test/GoogleTests.cs b/src/Security/Authentication/test/GoogleTests.cs index 1f2c4e3648c4..276284020cbe 100644 --- a/src/Security/Authentication/test/GoogleTests.cs +++ b/src/Security/Authentication/test/GoogleTests.cs @@ -53,18 +53,24 @@ public async Task ChallengeWillTriggerRedirection() using var server = host.GetTestServer(); var transaction = await server.SendAsync("https://example.com/challenge"); Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode); - var location = transaction.Response.Headers.Location.ToString(); - Assert.Contains("https://accounts.google.com/o/oauth2/v2/auth?response_type=code", location); - Assert.Contains("&client_id=", location); - Assert.Contains("&redirect_uri=", location); - Assert.Contains("&scope=", location); - Assert.Contains("&state=", location); - - Assert.DoesNotContain("access_type=", location); - Assert.DoesNotContain("prompt=", location); - Assert.DoesNotContain("approval_prompt=", location); - Assert.DoesNotContain("login_hint=", location); - Assert.DoesNotContain("include_granted_scopes=", location); + var location = transaction.Response.Headers.Location; + + Assert.StartsWith("https://accounts.google.com/o/oauth2/v2/auth?", location.AbsoluteUri); + + var queryParams = QueryHelpers.ParseQuery(location.Query); + Assert.Equal("code", queryParams["response_type"]); + Assert.Equal("Test Id", queryParams["client_id"]); + Assert.True(queryParams.ContainsKey("redirect_uri")); + Assert.True(queryParams.ContainsKey("scope")); + Assert.True(queryParams.ContainsKey("state")); + Assert.True(queryParams.ContainsKey("code_challenge")); + Assert.Equal("S256", queryParams["code_challenge_method"]); + + Assert.False(queryParams.ContainsKey("access_type")); + Assert.False(queryParams.ContainsKey("prompt")); + Assert.False(queryParams.ContainsKey("approval_prompt")); + Assert.False(queryParams.ContainsKey("login_hint")); + Assert.False(queryParams.ContainsKey("include_granted_scopes")); } [Fact] @@ -1010,6 +1016,75 @@ public async Task ChallengeGoogleWhenAlreadySignedWithGoogleSucceeds() Assert.StartsWith("https://www.facebook.com/", transaction.Response.Headers.Location.OriginalString); } + [Fact] + public async Task PkceSentToTokenEndpoint() + { + using var host = await CreateHost(o => + { + o.ClientId = "Test Client Id"; + o.ClientSecret = "Test Client Secret"; + o.BackchannelHttpHandler = new TestHttpMessageHandler + { + Sender = req => + { + if (req.RequestUri.AbsoluteUri == "https://oauth2.googleapis.com/token") + { + var body = req.Content.ReadAsStringAsync().Result; + var form = new FormReader(body); + var entries = form.ReadForm(); + Assert.Equal("Test Client Id", entries["client_id"]); + Assert.Equal("https://example.com/signin-google", entries["redirect_uri"]); + Assert.Equal("Test Client Secret", entries["client_secret"]); + Assert.Equal("TestCode", entries["code"]); + Assert.Equal("authorization_code", entries["grant_type"]); + Assert.False(string.IsNullOrEmpty(entries["code_verifier"])); + + return ReturnJsonResponse(new + { + access_token = "Test Access Token", + expire_in = 3600, + token_type = "Bearer", + }); + } + else if (req.RequestUri.GetComponents(UriComponents.SchemeAndServer | UriComponents.Path, UriFormat.UriEscaped) == "https://www.googleapis.com/oauth2/v3/userinfo") + { + return ReturnJsonResponse(new + { + id = "Test User ID", + displayName = "Test Name", + givenName = "Test Given Name", + surname = "Test Family Name", + mail = "Test email" + }); + } + + return null; + } + }; + }); + using var server = host.GetTestServer(); + var transaction = await server.SendAsync("https://example.com/challenge"); + Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode); + var locationUri = transaction.Response.Headers.Location; + Assert.StartsWith("https://accounts.google.com/o/oauth2/v2/auth", locationUri.AbsoluteUri); + + var queryParams = QueryHelpers.ParseQuery(locationUri.Query); + Assert.False(string.IsNullOrEmpty(queryParams["code_challenge"])); + Assert.Equal("S256", queryParams["code_challenge_method"]); + + var nonceCookie = transaction.SetCookie.Single(); + nonceCookie = nonceCookie.Substring(0, nonceCookie.IndexOf(';')); + + transaction = await server.SendAsync( + "https://example.com/signin-google?code=TestCode&state=" + queryParams["state"], + nonceCookie); + Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode); + Assert.Equal("/challenge", transaction.Response.Headers.GetValues("Location").First()); + Assert.Equal(2, transaction.SetCookie.Count); + Assert.StartsWith(".AspNetCore.Correlation.", transaction.SetCookie[0]); + Assert.StartsWith(".AspNetCore." + TestExtensions.CookieAuthenticationScheme, transaction.SetCookie[1]); + } + private HttpMessageHandler CreateBackchannel() { return new TestHttpMessageHandler()