Skip to content

Commit afb6ec4

Browse files
FranklinWhaleFranklinWhale
authored
Enable UsePkce by default for Google and Facebook auth (#45235)
1 parent 5fb23e3 commit afb6ec4

File tree

5 files changed

+198
-31
lines changed

5 files changed

+198
-31
lines changed

‎src/Security/Authentication/Facebook/src/FacebookOptions.cs

+1
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ public FacebookOptions()
2323
AuthorizationEndpoint = FacebookDefaults.AuthorizationEndpoint;
2424
TokenEndpoint = FacebookDefaults.TokenEndpoint;
2525
UserInformationEndpoint = FacebookDefaults.UserInformationEndpoint;
26+
UsePkce = true;
2627
Scope.Add("email");
2728
Fields.Add("name");
2829
Fields.Add("email");

‎src/Security/Authentication/Google/src/GoogleHandler.cs

+17-19
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
using Microsoft.AspNetCore.WebUtilities;
1212
using Microsoft.Extensions.Logging;
1313
using Microsoft.Extensions.Options;
14+
using Microsoft.Extensions.Primitives;
1415

1516
namespace Microsoft.AspNetCore.Authentication.Google;
1617

@@ -58,27 +59,24 @@ protected override string BuildChallengeUrl(AuthenticationProperties properties,
5859
// Google Identity Platform Manual:
5960
// https://developers.google.com/identity/protocols/OAuth2WebServer
6061

61-
var queryStrings = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
62-
queryStrings.Add("response_type", "code");
63-
queryStrings.Add("client_id", Options.ClientId);
64-
queryStrings.Add("redirect_uri", redirectUri);
62+
// Some query params and features (e.g. PKCE) are handled by the base class but some params have to be modified or added here
63+
var queryStrings = QueryHelpers.ParseQuery(new Uri(base.BuildChallengeUrl(properties, redirectUri)).Query);
6564

66-
AddQueryString(queryStrings, properties, GoogleChallengeProperties.ScopeKey, FormatScope, Options.Scope);
67-
AddQueryString(queryStrings, properties, GoogleChallengeProperties.AccessTypeKey, Options.AccessType);
68-
AddQueryString(queryStrings, properties, GoogleChallengeProperties.ApprovalPromptKey);
69-
AddQueryString(queryStrings, properties, GoogleChallengeProperties.PromptParameterKey);
70-
AddQueryString(queryStrings, properties, GoogleChallengeProperties.LoginHintKey);
71-
AddQueryString(queryStrings, properties, GoogleChallengeProperties.IncludeGrantedScopesKey, v => v?.ToString(CultureInfo.InvariantCulture).ToLowerInvariant(), (bool?)null);
65+
SetQueryParam(queryStrings, properties, GoogleChallengeProperties.ScopeKey, FormatScope, Options.Scope);
66+
SetQueryParam(queryStrings, properties, GoogleChallengeProperties.AccessTypeKey, Options.AccessType);
67+
SetQueryParam(queryStrings, properties, GoogleChallengeProperties.ApprovalPromptKey);
68+
SetQueryParam(queryStrings, properties, GoogleChallengeProperties.PromptParameterKey);
69+
SetQueryParam(queryStrings, properties, GoogleChallengeProperties.LoginHintKey);
70+
SetQueryParam(queryStrings, properties, GoogleChallengeProperties.IncludeGrantedScopesKey, v => v?.ToString(CultureInfo.InvariantCulture).ToLowerInvariant(), (bool?)null);
7271

73-
var state = Options.StateDataFormat.Protect(properties);
74-
queryStrings.Add("state", state);
72+
// Some properties are removed when setting query params above, so the state has to be reset
73+
queryStrings["state"] = Options.StateDataFormat.Protect(properties);
7574

76-
var authorizationEndpoint = QueryHelpers.AddQueryString(Options.AuthorizationEndpoint, queryStrings!);
77-
return authorizationEndpoint;
75+
return QueryHelpers.AddQueryString(Options.AuthorizationEndpoint, queryStrings);
7876
}
7977

80-
private static void AddQueryString<T>(
81-
IDictionary<string, string> queryStrings,
78+
private static void SetQueryParam<T>(
79+
IDictionary<string, StringValues> queryStrings,
8280
AuthenticationProperties properties,
8381
string name,
8482
Func<T, string?> formatter,
@@ -104,10 +102,10 @@ private static void AddQueryString<T>(
104102
}
105103
}
106104

107-
private static void AddQueryString(
108-
IDictionary<string, string> queryStrings,
105+
private static void SetQueryParam(
106+
IDictionary<string, StringValues> queryStrings,
109107
AuthenticationProperties properties,
110108
string name,
111109
string? defaultValue = null)
112-
=> AddQueryString(queryStrings, properties, name, x => x, defaultValue);
110+
=> SetQueryParam(queryStrings, properties, name, x => x, defaultValue);
113111
}

‎src/Security/Authentication/Google/src/GoogleOptions.cs

+1
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ public GoogleOptions()
2121
AuthorizationEndpoint = GoogleDefaults.AuthorizationEndpoint;
2222
TokenEndpoint = GoogleDefaults.TokenEndpoint;
2323
UserInformationEndpoint = GoogleDefaults.UserInformationEndpoint;
24+
UsePkce = true;
2425
Scope.Add("openid");
2526
Scope.Add("profile");
2627
Scope.Add("email");

‎src/Security/Authentication/test/FacebookTests.cs

+92
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
using Microsoft.AspNetCore.Hosting;
1313
using Microsoft.AspNetCore.Http;
1414
using Microsoft.AspNetCore.TestHost;
15+
using Microsoft.AspNetCore.WebUtilities;
1516
using Microsoft.Extensions.DependencyInjection;
1617
using Microsoft.Extensions.Hosting;
1718
using Microsoft.Extensions.Logging.Abstractions;
@@ -303,6 +304,8 @@ public async Task ChallengeWillTriggerRedirection()
303304
Assert.Contains("redirect_uri=", location);
304305
Assert.Contains("scope=", location);
305306
Assert.Contains("state=", location);
307+
Assert.Contains("code_challenge=", location);
308+
Assert.Contains("code_challenge_method=S256", location);
306309
}
307310

308311
[Fact]
@@ -368,6 +371,87 @@ public async Task CustomUserInfoEndpointHasValidGraphQuery()
368371
Assert.Contains("&appsecret_proof=b7fb6d5a4510926b4af6fe080497827d791dc45fe6541d88ba77bdf6e8e208c6&", finalUserInfoEndpoint);
369372
}
370373

374+
[Fact]
375+
public async Task PkceSentToTokenEndpoint()
376+
{
377+
using var host = await CreateHost(
378+
app => app.UseAuthentication(),
379+
services =>
380+
{
381+
services.AddAuthentication(TestExtensions.CookieAuthenticationScheme)
382+
.AddCookie(TestExtensions.CookieAuthenticationScheme)
383+
.AddFacebook(o =>
384+
{
385+
o.AppId = "Test App Id";
386+
o.AppSecret = "Test App Secret";
387+
o.BackchannelHttpHandler = new TestHttpMessageHandler
388+
{
389+
Sender = req =>
390+
{
391+
if (req.RequestUri.AbsoluteUri == "https://graph.facebook.com/v14.0/oauth/access_token")
392+
{
393+
var body = req.Content.ReadAsStringAsync().Result;
394+
var form = new FormReader(body);
395+
var entries = form.ReadForm();
396+
Assert.Equal("Test App Id", entries["client_id"]);
397+
Assert.Equal("https://example.com/signin-facebook", entries["redirect_uri"]);
398+
Assert.Equal("Test App Secret", entries["client_secret"]);
399+
Assert.Equal("TestCode", entries["code"]);
400+
Assert.Equal("authorization_code", entries["grant_type"]);
401+
Assert.False(string.IsNullOrEmpty(entries["code_verifier"]));
402+
403+
return ReturnJsonResponse(new
404+
{
405+
access_token = "Test Access Token",
406+
expire_in = 3600,
407+
token_type = "Bearer",
408+
});
409+
}
410+
else if (req.RequestUri.GetComponents(UriComponents.SchemeAndServer | UriComponents.Path, UriFormat.UriEscaped) == "https://graph.facebook.com/v14.0/me")
411+
{
412+
return ReturnJsonResponse(new
413+
{
414+
id = "Test User ID",
415+
displayName = "Test Name",
416+
givenName = "Test Given Name",
417+
surname = "Test Family Name",
418+
mail = "Test email"
419+
});
420+
}
421+
422+
return null;
423+
}
424+
};
425+
});
426+
},
427+
async context =>
428+
{
429+
await context.ChallengeAsync("Facebook");
430+
return true;
431+
});
432+
using var server = host.GetTestServer();
433+
var transaction = await server.SendAsync("https://example.com/challenge");
434+
Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode);
435+
var locationUri = transaction.Response.Headers.Location;
436+
Assert.StartsWith("https://www.facebook.com/v14.0/dialog/oauth", locationUri.AbsoluteUri);
437+
438+
var queryParams = QueryHelpers.ParseQuery(locationUri.Query);
439+
Assert.False(string.IsNullOrEmpty(queryParams["code_challenge"]));
440+
Assert.Equal("S256", queryParams["code_challenge_method"]);
441+
442+
var nonceCookie = transaction.SetCookie.Single();
443+
nonceCookie = nonceCookie.Substring(0, nonceCookie.IndexOf(';'));
444+
445+
transaction = await server.SendAsync(
446+
"https://example.com/signin-facebook?code=TestCode&state=" + queryParams["state"],
447+
nonceCookie);
448+
Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode);
449+
Assert.Equal("/challenge", transaction.Response.Headers.GetValues("Location").First());
450+
Assert.Equal(2, transaction.SetCookie.Count);
451+
Assert.StartsWith(".AspNetCore.Correlation.", transaction.SetCookie[0]);
452+
Assert.StartsWith(".AspNetCore." + TestExtensions.CookieAuthenticationScheme, transaction.SetCookie[1]);
453+
}
454+
371455
private static async Task<IHost> CreateHost(Action<IApplicationBuilder> configure, Action<IServiceCollection> configureServices, Func<HttpContext, Task<bool>> handler)
372456
{
373457
var host = new HostBuilder()
@@ -390,4 +474,12 @@ private static async Task<IHost> CreateHost(Action<IApplicationBuilder> configur
390474
await host.StartAsync();
391475
return host;
392476
}
477+
478+
private static HttpResponseMessage ReturnJsonResponse(object content, HttpStatusCode code = HttpStatusCode.OK)
479+
{
480+
var res = new HttpResponseMessage(code);
481+
var text = Newtonsoft.Json.JsonConvert.SerializeObject(content);
482+
res.Content = new StringContent(text, Encoding.UTF8, "application/json");
483+
return res;
484+
}
393485
}

‎src/Security/Authentication/test/GoogleTests.cs

+87-12
Original file line numberDiff line numberDiff line change
@@ -53,18 +53,24 @@ public async Task ChallengeWillTriggerRedirection()
5353
using var server = host.GetTestServer();
5454
var transaction = await server.SendAsync("https://example.com/challenge");
5555
Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode);
56-
var location = transaction.Response.Headers.Location.ToString();
57-
Assert.Contains("https://accounts.google.com/o/oauth2/v2/auth?response_type=code", location);
58-
Assert.Contains("&client_id=", location);
59-
Assert.Contains("&redirect_uri=", location);
60-
Assert.Contains("&scope=", location);
61-
Assert.Contains("&state=", location);
62-
63-
Assert.DoesNotContain("access_type=", location);
64-
Assert.DoesNotContain("prompt=", location);
65-
Assert.DoesNotContain("approval_prompt=", location);
66-
Assert.DoesNotContain("login_hint=", location);
67-
Assert.DoesNotContain("include_granted_scopes=", location);
56+
var location = transaction.Response.Headers.Location;
57+
58+
Assert.StartsWith("https://accounts.google.com/o/oauth2/v2/auth?", location.AbsoluteUri);
59+
60+
var queryParams = QueryHelpers.ParseQuery(location.Query);
61+
Assert.Equal("code", queryParams["response_type"]);
62+
Assert.Equal("Test Id", queryParams["client_id"]);
63+
Assert.True(queryParams.ContainsKey("redirect_uri"));
64+
Assert.True(queryParams.ContainsKey("scope"));
65+
Assert.True(queryParams.ContainsKey("state"));
66+
Assert.True(queryParams.ContainsKey("code_challenge"));
67+
Assert.Equal("S256", queryParams["code_challenge_method"]);
68+
69+
Assert.False(queryParams.ContainsKey("access_type"));
70+
Assert.False(queryParams.ContainsKey("prompt"));
71+
Assert.False(queryParams.ContainsKey("approval_prompt"));
72+
Assert.False(queryParams.ContainsKey("login_hint"));
73+
Assert.False(queryParams.ContainsKey("include_granted_scopes"));
6874
}
6975

7076
[Fact]
@@ -1010,6 +1016,75 @@ public async Task ChallengeGoogleWhenAlreadySignedWithGoogleSucceeds()
10101016
Assert.StartsWith("https://www.facebook.com/", transaction.Response.Headers.Location.OriginalString);
10111017
}
10121018

1019+
[Fact]
1020+
public async Task PkceSentToTokenEndpoint()
1021+
{
1022+
using var host = await CreateHost(o =>
1023+
{
1024+
o.ClientId = "Test Client Id";
1025+
o.ClientSecret = "Test Client Secret";
1026+
o.BackchannelHttpHandler = new TestHttpMessageHandler
1027+
{
1028+
Sender = req =>
1029+
{
1030+
if (req.RequestUri.AbsoluteUri == "https://oauth2.googleapis.com/token")
1031+
{
1032+
var body = req.Content.ReadAsStringAsync().Result;
1033+
var form = new FormReader(body);
1034+
var entries = form.ReadForm();
1035+
Assert.Equal("Test Client Id", entries["client_id"]);
1036+
Assert.Equal("https://example.com/signin-google", entries["redirect_uri"]);
1037+
Assert.Equal("Test Client Secret", entries["client_secret"]);
1038+
Assert.Equal("TestCode", entries["code"]);
1039+
Assert.Equal("authorization_code", entries["grant_type"]);
1040+
Assert.False(string.IsNullOrEmpty(entries["code_verifier"]));
1041+
1042+
return ReturnJsonResponse(new
1043+
{
1044+
access_token = "Test Access Token",
1045+
expire_in = 3600,
1046+
token_type = "Bearer",
1047+
});
1048+
}
1049+
else if (req.RequestUri.GetComponents(UriComponents.SchemeAndServer | UriComponents.Path, UriFormat.UriEscaped) == "https://www.googleapis.com/oauth2/v3/userinfo")
1050+
{
1051+
return ReturnJsonResponse(new
1052+
{
1053+
id = "Test User ID",
1054+
displayName = "Test Name",
1055+
givenName = "Test Given Name",
1056+
surname = "Test Family Name",
1057+
mail = "Test email"
1058+
});
1059+
}
1060+
1061+
return null;
1062+
}
1063+
};
1064+
});
1065+
using var server = host.GetTestServer();
1066+
var transaction = await server.SendAsync("https://example.com/challenge");
1067+
Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode);
1068+
var locationUri = transaction.Response.Headers.Location;
1069+
Assert.StartsWith("https://accounts.google.com/o/oauth2/v2/auth", locationUri.AbsoluteUri);
1070+
1071+
var queryParams = QueryHelpers.ParseQuery(locationUri.Query);
1072+
Assert.False(string.IsNullOrEmpty(queryParams["code_challenge"]));
1073+
Assert.Equal("S256", queryParams["code_challenge_method"]);
1074+
1075+
var nonceCookie = transaction.SetCookie.Single();
1076+
nonceCookie = nonceCookie.Substring(0, nonceCookie.IndexOf(';'));
1077+
1078+
transaction = await server.SendAsync(
1079+
"https://example.com/signin-google?code=TestCode&state=" + queryParams["state"],
1080+
nonceCookie);
1081+
Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode);
1082+
Assert.Equal("/challenge", transaction.Response.Headers.GetValues("Location").First());
1083+
Assert.Equal(2, transaction.SetCookie.Count);
1084+
Assert.StartsWith(".AspNetCore.Correlation.", transaction.SetCookie[0]);
1085+
Assert.StartsWith(".AspNetCore." + TestExtensions.CookieAuthenticationScheme, transaction.SetCookie[1]);
1086+
}
1087+
10131088
private HttpMessageHandler CreateBackchannel()
10141089
{
10151090
return new TestHttpMessageHandler()

0 commit comments

Comments
 (0)