Enable UsePkce by default for Google and Facebook auth

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");

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<string, string>(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<T>(
-        IDictionary<string, string> queryStrings,
+    private static void SetQueryParam<T>(
+        IDictionary<string, StringValues> queryStrings,
         AuthenticationProperties properties,
         string name,
         Func<T, string?> formatter,
@@ -104,10 +102,10 @@ private static void AddQueryString<T>(
         }
     }
 
-    private static void AddQueryString(
-        IDictionary<string, string> queryStrings,
+    private static void SetQueryParam(
+        IDictionary<string, StringValues> queryStrings,
         AuthenticationProperties properties,
         string name,
         string? defaultValue = null)
-        => AddQueryString(queryStrings, properties, name, x => x, defaultValue);
+        => SetQueryParam(queryStrings, properties, name, x => x, defaultValue);
 }

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");

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<IHost> CreateHost(Action<IApplicationBuilder> configure, Action<IServiceCollection> configureServices, Func<HttpContext, Task<bool>> handler)
     {
         var host = new HostBuilder()
@@ -390,4 +474,12 @@ private static async Task<IHost> CreateHost(Action<IApplicationBuilder> 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;
+    }
 }

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()