From 5104413a764a1c77cf32136f996a2ce97b41e54d Mon Sep 17 00:00:00 2001
From: Franklin Tse <FranklinWhale@users.noreply.github.com>
Date: Tue, 22 Nov 2022 17:41:01 +0800
Subject: [PATCH 01/18] Add `UsePkce = true`

---
 src/Security/Authentication/Facebook/src/FacebookOptions.cs | 1 +
 src/Security/Authentication/Google/src/GoogleOptions.cs     | 1 +
 2 files changed, 2 insertions(+)

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

From 04c82b16331ea8945307b07540dd874b6ab4acbc Mon Sep 17 00:00:00 2001
From: Franklin Tse <FranklinWhale@users.noreply.github.com>
Date: Tue, 22 Nov 2022 17:53:06 +0800
Subject: [PATCH 02/18] Update `ChallengeWillTriggerRedirection`

---
 src/Security/Authentication/test/FacebookTests.cs | 2 ++
 src/Security/Authentication/test/GoogleTests.cs   | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/src/Security/Authentication/test/FacebookTests.cs b/src/Security/Authentication/test/FacebookTests.cs
index 001fea35feea..0774a836dfce 100644
--- a/src/Security/Authentication/test/FacebookTests.cs
+++ b/src/Security/Authentication/test/FacebookTests.cs
@@ -303,6 +303,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]
diff --git a/src/Security/Authentication/test/GoogleTests.cs b/src/Security/Authentication/test/GoogleTests.cs
index 1f2c4e3648c4..71ff50919d5d 100644
--- a/src/Security/Authentication/test/GoogleTests.cs
+++ b/src/Security/Authentication/test/GoogleTests.cs
@@ -59,6 +59,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);
 
         Assert.DoesNotContain("access_type=", location);
         Assert.DoesNotContain("prompt=", location);

From 8030cbd22aaf04d265b00bf1aeba94a6e5015f02 Mon Sep 17 00:00:00 2001
From: Franklin Tse <FranklinWhale@users.noreply.github.com>
Date: Tue, 22 Nov 2022 18:27:06 +0800
Subject: [PATCH 03/18] Add `PkceSentToTokenEndpoint` in test

---
 .../Authentication/test/FacebookTests.cs      | 69 +++++++++++++++++++
 .../Authentication/test/GoogleTests.cs        | 69 +++++++++++++++++++
 2 files changed, 138 insertions(+)

diff --git a/src/Security/Authentication/test/FacebookTests.cs b/src/Security/Authentication/test/FacebookTests.cs
index 0774a836dfce..5c0d7f816ba7 100644
--- a/src/Security/Authentication/test/FacebookTests.cs
+++ b/src/Security/Authentication/test/FacebookTests.cs
@@ -370,6 +370,75 @@ public async Task CustomUserInfoEndpointHasValidGraphQuery()
         Assert.Contains("&appsecret_proof=b7fb6d5a4510926b4af6fe080497827d791dc45fe6541d88ba77bdf6e8e208c6&", finalUserInfoEndpoint);
     }
 
+    [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://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 Client Id", entries["client_id"]);
+                        Assert.Equal("https://example.com/signin-facebook", 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://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;
+                }
+            };
+        });
+        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("/me", 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()
diff --git a/src/Security/Authentication/test/GoogleTests.cs b/src/Security/Authentication/test/GoogleTests.cs
index 71ff50919d5d..4e08abf4a3ef 100644
--- a/src/Security/Authentication/test/GoogleTests.cs
+++ b/src/Security/Authentication/test/GoogleTests.cs
@@ -1012,6 +1012,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("/me", 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()

From 3e9d8d62248bbd2865915ee2e0ab523d6f323b54 Mon Sep 17 00:00:00 2001
From: Franklin Tse <FranklinWhale@users.noreply.github.com>
Date: Tue, 22 Nov 2022 19:44:15 +0800
Subject: [PATCH 04/18] Fix `CreateHost` in Facebook

---
 .../Authentication/test/FacebookTests.cs      | 88 +++++++++++--------
 1 file changed, 50 insertions(+), 38 deletions(-)

diff --git a/src/Security/Authentication/test/FacebookTests.cs b/src/Security/Authentication/test/FacebookTests.cs
index 5c0d7f816ba7..c0397ee73baf 100644
--- a/src/Security/Authentication/test/FacebookTests.cs
+++ b/src/Security/Authentication/test/FacebookTests.cs
@@ -373,49 +373,61 @@ public async Task CustomUserInfoEndpointHasValidGraphQuery()
     [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
+        using var host = await CreateHost(
+            app => app.UseAuthentication(),
+            services =>
             {
-                Sender = req =>
+                services.AddAuthentication(CookieAuthenticationDefaults.AuthenticationScheme)
+                    .AddCookie()
+                    .AddFacebook(o =>
                 {
-                    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 Client Id", entries["client_id"]);
-                        Assert.Equal("https://example.com/signin-facebook", 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://graph.facebook.com/v14.0/me")
+                    o.AppId = "Test App Id";
+                    o.AppSecret = "Test App Secret";
+                    o.BackchannelHttpHandler = new TestHttpMessageHandler
                     {
-                        return ReturnJsonResponse(new
+                        Sender = req =>
                         {
-                            id = "Test User ID",
-                            displayName = "Test Name",
-                            givenName = "Test Given Name",
-                            surname = "Test Family Name",
-                            mail = "Test email"
-                        });
-                    }
+                            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 Client Id", entries["client_id"]);
+                                Assert.Equal("https://example.com/signin-facebook", 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 null;
-                }
-            };
-        });
+                                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);

From 6cb7dd9aeba1efd5144bd59fe7d5d43eeb758c46 Mon Sep 17 00:00:00 2001
From: Franklin Tse <FranklinWhale@users.noreply.github.com>
Date: Tue, 22 Nov 2022 21:33:37 +0800
Subject: [PATCH 05/18] Add missing items

---
 src/Security/Authentication/test/FacebookTests.cs | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/Security/Authentication/test/FacebookTests.cs b/src/Security/Authentication/test/FacebookTests.cs
index c0397ee73baf..ee0989830a50 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;
@@ -473,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;
+    }
 }

From 3a945bec3f069ec4edabf2f245a864f5a478f8d7 Mon Sep 17 00:00:00 2001
From: Franklin Tse <FranklinWhale@users.noreply.github.com>
Date: Tue, 22 Nov 2022 23:57:44 +0800
Subject: [PATCH 06/18] Fix `GoogleHandler` not handling `UsePkce`

---
 .../Authentication/Google/src/GoogleHandler.cs  | 13 +++----------
 .../Authentication/OAuth/src/OAuthHandler.cs    | 17 +++++++++++++----
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/src/Security/Authentication/Google/src/GoogleHandler.cs b/src/Security/Authentication/Google/src/GoogleHandler.cs
index beda9af2daf8..8efcd2421dd9 100644
--- a/src/Security/Authentication/Google/src/GoogleHandler.cs
+++ b/src/Security/Authentication/Google/src/GoogleHandler.cs
@@ -53,15 +53,12 @@ protected override async Task<AuthenticationTicket> CreateTicketAsync(
     }
 
     /// <inheritdoc />
-    protected override string BuildChallengeUrl(AuthenticationProperties properties, string redirectUri)
+    protected override IDictionary<string, string> BuildChallengeUrlQuery(AuthenticationProperties properties, string redirectUri)
     {
         // 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);
+        var queryStrings = base.BuildChallengeUrlQuery(properties, redirectUri);
 
         AddQueryString(queryStrings, properties, GoogleChallengeProperties.ScopeKey, FormatScope, Options.Scope);
         AddQueryString(queryStrings, properties, GoogleChallengeProperties.AccessTypeKey, Options.AccessType);
@@ -70,11 +67,7 @@ protected override string BuildChallengeUrl(AuthenticationProperties properties,
         AddQueryString(queryStrings, properties, GoogleChallengeProperties.LoginHintKey);
         AddQueryString(queryStrings, properties, GoogleChallengeProperties.IncludeGrantedScopesKey, v => v?.ToString(CultureInfo.InvariantCulture).ToLowerInvariant(), (bool?)null);
 
-        var state = Options.StateDataFormat.Protect(properties);
-        queryStrings.Add("state", state);
-
-        var authorizationEndpoint = QueryHelpers.AddQueryString(Options.AuthorizationEndpoint, queryStrings!);
-        return authorizationEndpoint;
+        return queryStrings;
     }
 
     private static void AddQueryString<T>(
diff --git a/src/Security/Authentication/OAuth/src/OAuthHandler.cs b/src/Security/Authentication/OAuth/src/OAuthHandler.cs
index 31f3079634ed..6aba738469dc 100644
--- a/src/Security/Authentication/OAuth/src/OAuthHandler.cs
+++ b/src/Security/Authentication/OAuth/src/OAuthHandler.cs
@@ -283,12 +283,12 @@ protected override async Task HandleChallengeAsync(AuthenticationProperties prop
     }
 
     /// <summary>
-    /// Constructs the OAuth challenge url.
+    /// Constructs the query of the OAuth challenge url.
     /// </summary>
     /// <param name="properties">The <see cref="AuthenticationProperties"/>.</param>
     /// <param name="redirectUri">The url to redirect to once the challenge is completed.</param>
-    /// <returns>The challenge url.</returns>
-    protected virtual string BuildChallengeUrl(AuthenticationProperties properties, string redirectUri)
+    /// <returns>The challenge url query.</returns>
+    protected virtual IDictionary<string, string> BuildChallengeUrlQuery(AuthenticationProperties properties, string redirectUri)
     {
         var scopeParameter = properties.GetParameter<ICollection<string>>(OAuthChallengeProperties.ScopeKey);
         var scope = scopeParameter != null ? FormatScope(scopeParameter) : FormatScope();
@@ -319,9 +319,18 @@ protected virtual string BuildChallengeUrl(AuthenticationProperties properties,
 
         parameters["state"] = Options.StateDataFormat.Protect(properties);
 
-        return QueryHelpers.AddQueryString(Options.AuthorizationEndpoint, parameters!);
+        return parameters;
     }
 
+    /// <summary>
+    /// Constructs the OAuth challenge url.
+    /// </summary>
+    /// <param name="properties">The <see cref="AuthenticationProperties"/>.</param>
+    /// <param name="redirectUri">The url to redirect to once the challenge is completed.</param>
+    /// <returns>The challenge url.</returns>
+    protected virtual string BuildChallengeUrl(AuthenticationProperties properties, string redirectUri) =>
+        QueryHelpers.AddQueryString(Options.AuthorizationEndpoint, BuildChallengeUrlQuery(properties, redirectUri));
+
     /// <summary>
     /// Format a list of OAuth scopes.
     /// </summary>

From aff69638f25ba850bf3407c5b4a73a1c94a455d5 Mon Sep 17 00:00:00 2001
From: Franklin Tse <FranklinWhale@users.noreply.github.com>
Date: Tue, 22 Nov 2022 23:58:56 +0800
Subject: [PATCH 07/18] Fix test

---
 src/Security/Authentication/test/FacebookTests.cs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/Security/Authentication/test/FacebookTests.cs b/src/Security/Authentication/test/FacebookTests.cs
index ee0989830a50..0f89b0a19e24 100644
--- a/src/Security/Authentication/test/FacebookTests.cs
+++ b/src/Security/Authentication/test/FacebookTests.cs
@@ -393,9 +393,9 @@ public async Task PkceSentToTokenEndpoint()
                                 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("Test App Id", entries["client_id"]);
                                 Assert.Equal("https://example.com/signin-facebook", entries["redirect_uri"]);
-                                Assert.Equal("Test Client Secret", entries["client_secret"]);
+                                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"]));

From 0e161809f3b06dfaaa3493d8703fb67634e3ea3e Mon Sep 17 00:00:00 2001
From: Franklin Tse <FranklinWhale@users.noreply.github.com>
Date: Wed, 23 Nov 2022 00:09:00 +0800
Subject: [PATCH 08/18] `IDictionary<string,string?>`

---
 src/Security/Authentication/Google/src/GoogleHandler.cs | 2 +-
 src/Security/Authentication/OAuth/src/OAuthHandler.cs   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/Security/Authentication/Google/src/GoogleHandler.cs b/src/Security/Authentication/Google/src/GoogleHandler.cs
index 8efcd2421dd9..ac937f8a5923 100644
--- a/src/Security/Authentication/Google/src/GoogleHandler.cs
+++ b/src/Security/Authentication/Google/src/GoogleHandler.cs
@@ -53,7 +53,7 @@ protected override async Task<AuthenticationTicket> CreateTicketAsync(
     }
 
     /// <inheritdoc />
-    protected override IDictionary<string, string> BuildChallengeUrlQuery(AuthenticationProperties properties, string redirectUri)
+    protected override IDictionary<string, string?> BuildChallengeUrlQuery(AuthenticationProperties properties, string redirectUri)
     {
         // Google Identity Platform Manual:
         // https://developers.google.com/identity/protocols/OAuth2WebServer
diff --git a/src/Security/Authentication/OAuth/src/OAuthHandler.cs b/src/Security/Authentication/OAuth/src/OAuthHandler.cs
index 6aba738469dc..9eecd4b4ee50 100644
--- a/src/Security/Authentication/OAuth/src/OAuthHandler.cs
+++ b/src/Security/Authentication/OAuth/src/OAuthHandler.cs
@@ -288,7 +288,7 @@ protected override async Task HandleChallengeAsync(AuthenticationProperties prop
     /// <param name="properties">The <see cref="AuthenticationProperties"/>.</param>
     /// <param name="redirectUri">The url to redirect to once the challenge is completed.</param>
     /// <returns>The challenge url query.</returns>
-    protected virtual IDictionary<string, string> BuildChallengeUrlQuery(AuthenticationProperties properties, string redirectUri)
+    protected virtual IDictionary<string, string?> BuildChallengeUrlQuery(AuthenticationProperties properties, string redirectUri)
     {
         var scopeParameter = properties.GetParameter<ICollection<string>>(OAuthChallengeProperties.ScopeKey);
         var scope = scopeParameter != null ? FormatScope(scopeParameter) : FormatScope();

From 9511937f04c8fb4d0896067da4f537ef4d90b1a7 Mon Sep 17 00:00:00 2001
From: Franklin Tse <FranklinWhale@users.noreply.github.com>
Date: Wed, 23 Nov 2022 00:19:51 +0800
Subject: [PATCH 09/18] Actually `IDictionary<string, string>` is the better
 choice

---
 src/Security/Authentication/Google/src/GoogleHandler.cs | 2 +-
 src/Security/Authentication/OAuth/src/OAuthHandler.cs   | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/Security/Authentication/Google/src/GoogleHandler.cs b/src/Security/Authentication/Google/src/GoogleHandler.cs
index ac937f8a5923..8efcd2421dd9 100644
--- a/src/Security/Authentication/Google/src/GoogleHandler.cs
+++ b/src/Security/Authentication/Google/src/GoogleHandler.cs
@@ -53,7 +53,7 @@ protected override async Task<AuthenticationTicket> CreateTicketAsync(
     }
 
     /// <inheritdoc />
-    protected override IDictionary<string, string?> BuildChallengeUrlQuery(AuthenticationProperties properties, string redirectUri)
+    protected override IDictionary<string, string> BuildChallengeUrlQuery(AuthenticationProperties properties, string redirectUri)
     {
         // Google Identity Platform Manual:
         // https://developers.google.com/identity/protocols/OAuth2WebServer
diff --git a/src/Security/Authentication/OAuth/src/OAuthHandler.cs b/src/Security/Authentication/OAuth/src/OAuthHandler.cs
index 9eecd4b4ee50..698930a271ad 100644
--- a/src/Security/Authentication/OAuth/src/OAuthHandler.cs
+++ b/src/Security/Authentication/OAuth/src/OAuthHandler.cs
@@ -288,7 +288,7 @@ protected override async Task HandleChallengeAsync(AuthenticationProperties prop
     /// <param name="properties">The <see cref="AuthenticationProperties"/>.</param>
     /// <param name="redirectUri">The url to redirect to once the challenge is completed.</param>
     /// <returns>The challenge url query.</returns>
-    protected virtual IDictionary<string, string?> BuildChallengeUrlQuery(AuthenticationProperties properties, string redirectUri)
+    protected virtual IDictionary<string, string> BuildChallengeUrlQuery(AuthenticationProperties properties, string redirectUri)
     {
         var scopeParameter = properties.GetParameter<ICollection<string>>(OAuthChallengeProperties.ScopeKey);
         var scope = scopeParameter != null ? FormatScope(scopeParameter) : FormatScope();
@@ -329,7 +329,7 @@ protected override async Task HandleChallengeAsync(AuthenticationProperties prop
     /// <param name="redirectUri">The url to redirect to once the challenge is completed.</param>
     /// <returns>The challenge url.</returns>
     protected virtual string BuildChallengeUrl(AuthenticationProperties properties, string redirectUri) =>
-        QueryHelpers.AddQueryString(Options.AuthorizationEndpoint, BuildChallengeUrlQuery(properties, redirectUri));
+        QueryHelpers.AddQueryString(Options.AuthorizationEndpoint, BuildChallengeUrlQuery(properties, redirectUri)!);
 
     /// <summary>
     /// Format a list of OAuth scopes.

From 88ac8c744d79cecad367c7aa3d9c510e2b5f6e0d Mon Sep 17 00:00:00 2001
From: Franklin Tse <FranklinWhale@users.noreply.github.com>
Date: Wed, 23 Nov 2022 19:52:38 +0800
Subject: [PATCH 10/18] No new api

---
 .../Authentication/Google/src/GoogleHandler.cs  | 10 +++++-----
 .../Authentication/OAuth/src/OAuthHandler.cs    | 17 ++++-------------
 2 files changed, 9 insertions(+), 18 deletions(-)

diff --git a/src/Security/Authentication/Google/src/GoogleHandler.cs b/src/Security/Authentication/Google/src/GoogleHandler.cs
index 8efcd2421dd9..304e58251b7e 100644
--- a/src/Security/Authentication/Google/src/GoogleHandler.cs
+++ b/src/Security/Authentication/Google/src/GoogleHandler.cs
@@ -53,12 +53,12 @@ protected override async Task<AuthenticationTicket> CreateTicketAsync(
     }
 
     /// <inheritdoc />
-    protected override IDictionary<string, string> BuildChallengeUrlQuery(AuthenticationProperties properties, string redirectUri)
+    protected override string BuildChallengeUrl(AuthenticationProperties properties, string redirectUri)
     {
         // Google Identity Platform Manual:
         // https://developers.google.com/identity/protocols/OAuth2WebServer
 
-        var queryStrings = base.BuildChallengeUrlQuery(properties, redirectUri);
+        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);
@@ -67,11 +67,11 @@ protected override IDictionary<string, string> BuildChallengeUrlQuery(Authentica
         AddQueryString(queryStrings, properties, GoogleChallengeProperties.LoginHintKey);
         AddQueryString(queryStrings, properties, GoogleChallengeProperties.IncludeGrantedScopesKey, v => v?.ToString(CultureInfo.InvariantCulture).ToLowerInvariant(), (bool?)null);
 
-        return queryStrings;
+        return QueryHelpers.AddQueryString(Options.AuthorizationEndpoint, queryStrings!);
     }
 
     private static void AddQueryString<T>(
-        IDictionary<string, string> queryStrings,
+        IDictionary<string, StringValues> queryStrings,
         AuthenticationProperties properties,
         string name,
         Func<T, string?> formatter,
@@ -98,7 +98,7 @@ private static void AddQueryString<T>(
     }
 
     private static void AddQueryString(
-        IDictionary<string, string> queryStrings,
+        IDictionary<string, StringValues> queryStrings,
         AuthenticationProperties properties,
         string name,
         string? defaultValue = null)
diff --git a/src/Security/Authentication/OAuth/src/OAuthHandler.cs b/src/Security/Authentication/OAuth/src/OAuthHandler.cs
index 698930a271ad..31f3079634ed 100644
--- a/src/Security/Authentication/OAuth/src/OAuthHandler.cs
+++ b/src/Security/Authentication/OAuth/src/OAuthHandler.cs
@@ -283,12 +283,12 @@ protected override async Task HandleChallengeAsync(AuthenticationProperties prop
     }
 
     /// <summary>
-    /// Constructs the query of the OAuth challenge url.
+    /// Constructs the OAuth challenge url.
     /// </summary>
     /// <param name="properties">The <see cref="AuthenticationProperties"/>.</param>
     /// <param name="redirectUri">The url to redirect to once the challenge is completed.</param>
-    /// <returns>The challenge url query.</returns>
-    protected virtual IDictionary<string, string> BuildChallengeUrlQuery(AuthenticationProperties properties, string redirectUri)
+    /// <returns>The challenge url.</returns>
+    protected virtual string BuildChallengeUrl(AuthenticationProperties properties, string redirectUri)
     {
         var scopeParameter = properties.GetParameter<ICollection<string>>(OAuthChallengeProperties.ScopeKey);
         var scope = scopeParameter != null ? FormatScope(scopeParameter) : FormatScope();
@@ -319,18 +319,9 @@ protected virtual IDictionary<string, string> BuildChallengeUrlQuery(Authenticat
 
         parameters["state"] = Options.StateDataFormat.Protect(properties);
 
-        return parameters;
+        return QueryHelpers.AddQueryString(Options.AuthorizationEndpoint, parameters!);
     }
 
-    /// <summary>
-    /// Constructs the OAuth challenge url.
-    /// </summary>
-    /// <param name="properties">The <see cref="AuthenticationProperties"/>.</param>
-    /// <param name="redirectUri">The url to redirect to once the challenge is completed.</param>
-    /// <returns>The challenge url.</returns>
-    protected virtual string BuildChallengeUrl(AuthenticationProperties properties, string redirectUri) =>
-        QueryHelpers.AddQueryString(Options.AuthorizationEndpoint, BuildChallengeUrlQuery(properties, redirectUri)!);
-
     /// <summary>
     /// Format a list of OAuth scopes.
     /// </summary>

From 5e699082f361777678e578f36c7c2ead907c11ea Mon Sep 17 00:00:00 2001
From: Franklin Tse <FranklinWhale@users.noreply.github.com>
Date: Wed, 23 Nov 2022 19:59:22 +0800
Subject: [PATCH 11/18] null-forgiving operator no longer required

---
 src/Security/Authentication/Google/src/GoogleHandler.cs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Security/Authentication/Google/src/GoogleHandler.cs b/src/Security/Authentication/Google/src/GoogleHandler.cs
index 304e58251b7e..b51d8f047080 100644
--- a/src/Security/Authentication/Google/src/GoogleHandler.cs
+++ b/src/Security/Authentication/Google/src/GoogleHandler.cs
@@ -67,7 +67,7 @@ protected override string BuildChallengeUrl(AuthenticationProperties properties,
         AddQueryString(queryStrings, properties, GoogleChallengeProperties.LoginHintKey);
         AddQueryString(queryStrings, properties, GoogleChallengeProperties.IncludeGrantedScopesKey, v => v?.ToString(CultureInfo.InvariantCulture).ToLowerInvariant(), (bool?)null);
 
-        return QueryHelpers.AddQueryString(Options.AuthorizationEndpoint, queryStrings!);
+        return QueryHelpers.AddQueryString(Options.AuthorizationEndpoint, queryStrings);
     }
 
     private static void AddQueryString<T>(

From 0b2b44b121d0f9bc8aad9436e81c5a004a2265c7 Mon Sep 17 00:00:00 2001
From: Franklin Tse <FranklinWhale@users.noreply.github.com>
Date: Wed, 23 Nov 2022 20:17:31 +0800
Subject: [PATCH 12/18] Add namespace

---
 src/Security/Authentication/Google/src/GoogleHandler.cs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/Security/Authentication/Google/src/GoogleHandler.cs b/src/Security/Authentication/Google/src/GoogleHandler.cs
index b51d8f047080..99013d61293f 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;
 

From 6093be9bf16b83ab5b6a6721db945990a03fd167 Mon Sep 17 00:00:00 2001
From: Franklin Tse <FranklinWhale@users.noreply.github.com>
Date: Wed, 23 Nov 2022 22:54:08 +0800
Subject: [PATCH 13/18] Fix `state` and tests

---
 .../Google/src/GoogleHandler.cs               | 21 +++++++-----
 .../Authentication/test/FacebookTests.cs      |  2 +-
 .../Authentication/test/GoogleTests.cs        | 34 +++++++++++--------
 3 files changed, 32 insertions(+), 25 deletions(-)

diff --git a/src/Security/Authentication/Google/src/GoogleHandler.cs b/src/Security/Authentication/Google/src/GoogleHandler.cs
index 99013d61293f..c5475b077e2d 100644
--- a/src/Security/Authentication/Google/src/GoogleHandler.cs
+++ b/src/Security/Authentication/Google/src/GoogleHandler.cs
@@ -61,17 +61,20 @@ protected override string BuildChallengeUrl(AuthenticationProperties properties,
 
         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);
+
+        // Some properties are removed when setting query params above, so the state has to be reset
+        queryStrings["state"] = Options.StateDataFormat.Protect(properties);
 
         return QueryHelpers.AddQueryString(Options.AuthorizationEndpoint, queryStrings);
     }
 
-    private static void AddQueryString<T>(
+    private static void SetQueryParam<T>(
         IDictionary<string, StringValues> queryStrings,
         AuthenticationProperties properties,
         string name,
@@ -98,10 +101,10 @@ private static void AddQueryString<T>(
         }
     }
 
-    private static void AddQueryString(
+    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);
 }
diff --git a/src/Security/Authentication/test/FacebookTests.cs b/src/Security/Authentication/test/FacebookTests.cs
index 0f89b0a19e24..4423fc0e0b70 100644
--- a/src/Security/Authentication/test/FacebookTests.cs
+++ b/src/Security/Authentication/test/FacebookTests.cs
@@ -364,7 +364,7 @@ public async Task CustomUserInfoEndpointHasValidGraphQuery()
             "https://example.com/signin-facebook?code=TestCode&state=" + UrlEncoder.Default.Encode(state),
             $".AspNetCore.Correlation.{correlationValue}=N");
         Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode);
-        Assert.Equal("/me", transaction.Response.Headers.GetValues("Location").First());
+        Assert.Equal("/challenge", transaction.Response.Headers.GetValues("Location").First());
         Assert.Equal(1, finalUserInfoEndpoint.Count(c => c == '?'));
         Assert.Contains("fields=email,timezone,picture", finalUserInfoEndpoint);
         Assert.Contains("&access_token=", finalUserInfoEndpoint);
diff --git a/src/Security/Authentication/test/GoogleTests.cs b/src/Security/Authentication/test/GoogleTests.cs
index 4e08abf4a3ef..923d5f9d6574 100644
--- a/src/Security/Authentication/test/GoogleTests.cs
+++ b/src/Security/Authentication/test/GoogleTests.cs
@@ -53,20 +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.Contains("&code_challenge=", location);
-        Assert.Contains("&code_challenge_method=S256", 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?", locationUri.AbsoluteUri);
+
+        var queryParams = QueryHelpers.ParseQuery(locationUri.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]
@@ -1075,7 +1079,7 @@ public async Task PkceSentToTokenEndpoint()
             "https://example.com/signin-google?code=TestCode&state=" + queryParams["state"],
             nonceCookie);
         Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode);
-        Assert.Equal("/me", transaction.Response.Headers.GetValues("Location").First());
+        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]);

From 9dad61a18279e238bbd315e681de45f95a0322d0 Mon Sep 17 00:00:00 2001
From: Franklin Tse <FranklinWhale@users.noreply.github.com>
Date: Wed, 23 Nov 2022 23:06:04 +0800
Subject: [PATCH 14/18] Fix test

---
 src/Security/Authentication/test/FacebookTests.cs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/Security/Authentication/test/FacebookTests.cs b/src/Security/Authentication/test/FacebookTests.cs
index 4423fc0e0b70..bd337e94fd7d 100644
--- a/src/Security/Authentication/test/FacebookTests.cs
+++ b/src/Security/Authentication/test/FacebookTests.cs
@@ -364,7 +364,7 @@ public async Task CustomUserInfoEndpointHasValidGraphQuery()
             "https://example.com/signin-facebook?code=TestCode&state=" + UrlEncoder.Default.Encode(state),
             $".AspNetCore.Correlation.{correlationValue}=N");
         Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode);
-        Assert.Equal("/challenge", transaction.Response.Headers.GetValues("Location").First());
+        Assert.Equal("/me", transaction.Response.Headers.GetValues("Location").First());
         Assert.Equal(1, finalUserInfoEndpoint.Count(c => c == '?'));
         Assert.Contains("fields=email,timezone,picture", finalUserInfoEndpoint);
         Assert.Contains("&access_token=", finalUserInfoEndpoint);
@@ -446,7 +446,7 @@ public async Task PkceSentToTokenEndpoint()
             "https://example.com/signin-facebook?code=TestCode&state=" + queryParams["state"],
             nonceCookie);
         Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode);
-        Assert.Equal("/me", transaction.Response.Headers.GetValues("Location").First());
+        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]);

From 61b862fec0032d62c56cdb460e970defa79b0714 Mon Sep 17 00:00:00 2001
From: Franklin Tse <FranklinWhale@users.noreply.github.com>
Date: Wed, 23 Nov 2022 23:41:33 +0800
Subject: [PATCH 15/18] Fix test

---
 src/Security/Authentication/test/GoogleTests.cs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Security/Authentication/test/GoogleTests.cs b/src/Security/Authentication/test/GoogleTests.cs
index 923d5f9d6574..d0bca205d074 100644
--- a/src/Security/Authentication/test/GoogleTests.cs
+++ b/src/Security/Authentication/test/GoogleTests.cs
@@ -55,7 +55,7 @@ public async Task ChallengeWillTriggerRedirection()
         Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode);
         var location = transaction.Response.Headers.Location;
 
-        Assert.StartsWith("https://accounts.google.com/o/oauth2/v2/auth?", locationUri.AbsoluteUri);
+        Assert.StartsWith("https://accounts.google.com/o/oauth2/v2/auth?", location.AbsoluteUri);
 
         var queryParams = QueryHelpers.ParseQuery(locationUri.Query);
         Assert.Equal("code", queryParams["response_type"]);

From 2b1751d9ba1a4934fce2cf6142bc565b8ff10b63 Mon Sep 17 00:00:00 2001
From: Franklin Tse <FranklinWhale@users.noreply.github.com>
Date: Thu, 24 Nov 2022 00:18:04 +0800
Subject: [PATCH 16/18] Fix build error

---
 src/Security/Authentication/test/GoogleTests.cs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Security/Authentication/test/GoogleTests.cs b/src/Security/Authentication/test/GoogleTests.cs
index d0bca205d074..276284020cbe 100644
--- a/src/Security/Authentication/test/GoogleTests.cs
+++ b/src/Security/Authentication/test/GoogleTests.cs
@@ -57,7 +57,7 @@ public async Task ChallengeWillTriggerRedirection()
 
         Assert.StartsWith("https://accounts.google.com/o/oauth2/v2/auth?", location.AbsoluteUri);
 
-        var queryParams = QueryHelpers.ParseQuery(locationUri.Query);
+        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"));

From d7be5ac1761d3b6865a09bb735c1e06288a8d897 Mon Sep 17 00:00:00 2001
From: Franklin Tse <FranklinWhale@users.noreply.github.com>
Date: Thu, 24 Nov 2022 02:13:34 +0800
Subject: [PATCH 17/18] Fix test

---
 .../Authentication/test/FacebookTests.cs      | 78 ++++++++++---------
 1 file changed, 41 insertions(+), 37 deletions(-)

diff --git a/src/Security/Authentication/test/FacebookTests.cs b/src/Security/Authentication/test/FacebookTests.cs
index bd337e94fd7d..b2022543a006 100644
--- a/src/Security/Authentication/test/FacebookTests.cs
+++ b/src/Security/Authentication/test/FacebookTests.cs
@@ -378,51 +378,55 @@ public async Task PkceSentToTokenEndpoint()
             app => app.UseAuthentication(),
             services =>
             {
-                services.AddAuthentication(CookieAuthenticationDefaults.AuthenticationScheme)
+                services.AddAuthentication(o =>
+                {
+                    o.DefaultScheme = TestExtensions.CookieAuthenticationScheme;
+                    o.DefaultChallengeScheme = FacebookDefaults.AuthenticationScheme;
+                })
                     .AddCookie()
                     .AddFacebook(o =>
-                {
-                    o.AppId = "Test App Id";
-                    o.AppSecret = "Test App Secret";
-                    o.BackchannelHttpHandler = new TestHttpMessageHandler
                     {
-                        Sender = req =>
+                        o.AppId = "Test App Id";
+                        o.AppSecret = "Test App Secret";
+                        o.BackchannelHttpHandler = new TestHttpMessageHandler
                         {
-                            if (req.RequestUri.AbsoluteUri == "https://graph.facebook.com/v14.0/oauth/access_token")
+                            Sender = req =>
                             {
-                                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
+                                if (req.RequestUri.AbsoluteUri == "https://graph.facebook.com/v14.0/oauth/access_token")
                                 {
-                                    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
+                                    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")
                                 {
-                                    id = "Test User ID",
-                                    displayName = "Test Name",
-                                    givenName = "Test Given Name",
-                                    surname = "Test Family Name",
-                                    mail = "Test email"
-                                });
+                                    return ReturnJsonResponse(new
+                                    {
+                                        id = "Test User ID",
+                                        displayName = "Test Name",
+                                        givenName = "Test Given Name",
+                                        surname = "Test Family Name",
+                                        mail = "Test email"
+                                    });
+                                }
+
+                                return null;
                             }
-
-                            return null;
-                        }
-                    };
-                });
+                        };
+                    });
             },
             async context =>
             {

From 47e8ff1334856df28ca8f0f0fe3aca258308f707 Mon Sep 17 00:00:00 2001
From: Franklin Tse <FranklinWhale@users.noreply.github.com>
Date: Thu, 24 Nov 2022 11:45:33 +0800
Subject: [PATCH 18/18] Add comment and fix test

---
 src/Security/Authentication/Google/src/GoogleHandler.cs | 1 +
 src/Security/Authentication/test/FacebookTests.cs       | 8 ++------
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/src/Security/Authentication/Google/src/GoogleHandler.cs b/src/Security/Authentication/Google/src/GoogleHandler.cs
index c5475b077e2d..33c10bb30d62 100644
--- a/src/Security/Authentication/Google/src/GoogleHandler.cs
+++ b/src/Security/Authentication/Google/src/GoogleHandler.cs
@@ -59,6 +59,7 @@ protected override string BuildChallengeUrl(AuthenticationProperties properties,
         // Google Identity Platform Manual:
         // https://developers.google.com/identity/protocols/OAuth2WebServer
 
+        // 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);
 
         SetQueryParam(queryStrings, properties, GoogleChallengeProperties.ScopeKey, FormatScope, Options.Scope);
diff --git a/src/Security/Authentication/test/FacebookTests.cs b/src/Security/Authentication/test/FacebookTests.cs
index b2022543a006..61cd2de5cf8c 100644
--- a/src/Security/Authentication/test/FacebookTests.cs
+++ b/src/Security/Authentication/test/FacebookTests.cs
@@ -378,12 +378,8 @@ public async Task PkceSentToTokenEndpoint()
             app => app.UseAuthentication(),
             services =>
             {
-                services.AddAuthentication(o =>
-                {
-                    o.DefaultScheme = TestExtensions.CookieAuthenticationScheme;
-                    o.DefaultChallengeScheme = FacebookDefaults.AuthenticationScheme;
-                })
-                    .AddCookie()
+                services.AddAuthentication(TestExtensions.CookieAuthenticationScheme)
+                    .AddCookie(TestExtensions.CookieAuthenticationScheme)
                     .AddFacebook(o =>
                     {
                         o.AppId = "Test App Id";