From 8be27083f68cb0bc8b807cde8f70e098855d1991 Mon Sep 17 00:00:00 2001 From: Chris R Date: Tue, 4 Jun 2019 14:46:21 -0700 Subject: [PATCH 1/8] Implement PKCE for OIDC #7734 --- .../OpenIdConnect/src/OpenIdConnectHandler.cs | 30 +++++++++++++++- .../OpenIdConnect/src/OpenIdConnectOptions.cs | 7 ++++ .../OpenIdConnectChallengeTests.cs | 35 ++++++++++++++++++- 3 files changed, 70 insertions(+), 2 deletions(-) diff --git a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs index b874f9665247..2f6642a76dd0 100644 --- a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs +++ b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs @@ -15,6 +15,7 @@ using System.Text.Json; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Microsoft.Extensions.Primitives; @@ -30,8 +31,11 @@ namespace Microsoft.AspNetCore.Authentication.OpenIdConnect public class OpenIdConnectHandler : RemoteAuthenticationHandler, IAuthenticationSignOutHandler { private const string NonceProperty = "N"; - private const string HeaderValueEpocDate = "Thu, 01 Jan 1970 00:00:00 GMT"; + private const string CodeVerifierKey = "code_verifier"; + private const string CodeChallengeKey = "code_challenge"; + private const string CodeChallengeMethodKey = "code_challenge_method"; + private const string CodeChallengeMethodS256 = "S256"; private static readonly RandomNumberGenerator CryptoRandom = RandomNumberGenerator.Create(); @@ -366,6 +370,24 @@ private async Task HandleChallengeAsyncInternal(AuthenticationProperties propert Scope = string.Join(" ", properties.GetParameter>(OpenIdConnectParameterNames.Scope) ?? Options.Scope), }; + // https://tools.ietf.org/html/rfc7636 + if (Options.UsePkse) + { + var verifierBytes = new byte[32]; + CryptoRandom.GetBytes(verifierBytes); + var codeVerifier = WebEncoders.Base64UrlEncode(verifierBytes); + + // Store this for use during the code redemption. See RunAuthorizationCodeReceivedEventAsync. + properties.Items.Add(CodeVerifierKey, codeVerifier); + + using var sha256 = SHA256.Create(); + var challengeBytes = sha256.ComputeHash(Encoding.UTF8.GetBytes(codeVerifier)); + var codeChallenge = WebEncoders.Base64UrlEncode(challengeBytes); + + message.Parameters.Add(CodeChallengeKey, codeChallenge); + message.Parameters.Add(CodeChallengeMethodKey, CodeChallengeMethodS256); + } + // Add the 'max_age' parameter to the authentication request if MaxAge is not null. // See http://openid.net/specs/openid-connect-core-1_0.html#AuthRequest var maxAge = properties.GetParameter(OpenIdConnectParameterNames.MaxAge) ?? Options.MaxAge; @@ -1097,6 +1119,12 @@ private async Task RunAuthorizationCodeReceive RedirectUri = properties.Items[OpenIdConnectDefaults.RedirectUriForCodePropertiesKey] }; + // PKCE https://tools.ietf.org/html/rfc7636#section-4.5, see HandleChallengeAsyncInternal + if (properties.Items.TryGetValue(CodeVerifierKey, out var codeVerifier)) + { + tokenEndpointRequest.Parameters.Add(CodeVerifierKey, codeVerifier); + } + var context = new AuthorizationCodeReceivedContext(Context, Scheme, Options, properties) { ProtocolMessage = authorizationResponse, diff --git a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs index 398327303924..96389c36d088 100644 --- a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs +++ b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs @@ -299,6 +299,13 @@ public CookieBuilder NonceCookie set => _nonceCookieBuilder = value ?? throw new ArgumentNullException(nameof(value)); } + /// + /// Enables or disables the use of the Proof Key for Code Exchange (PKCE) standard. + /// See https://tools.ietf.org/html/rfc7636. + /// The default value is `true`. + /// + public bool UsePkse { get; set; } = true; + private class OpenIdConnectNonceCookieBuilder : RequestPathBaseCookieBuilder { private readonly OpenIdConnectOptions _options; diff --git a/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs b/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs index cbafc46223b8..f02d5d352bb4 100644 --- a/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs +++ b/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs @@ -48,6 +48,39 @@ public async Task ChallengeRedirectIsIssuedCorrectly() OpenIdConnectParameterNames.VersionTelemetry); } + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task ChallengeIncludesPkceIfRequested(bool include) + { + var settings = new TestSettings( + opt => + { + opt.Authority = TestServerBuilder.DefaultAuthority; + opt.AuthenticationMethod = OpenIdConnectRedirectBehavior.RedirectGet; + opt.ClientId = "Test Id"; + opt.UsePkse = include; + }); + + var server = settings.CreateTestServer(); + var transaction = await server.SendAsync(ChallengeEndpoint); + + var res = transaction.Response; + Assert.Equal(HttpStatusCode.Redirect, res.StatusCode); + Assert.NotNull(res.Headers.Location); + + if (include) + { + Assert.Contains("code_challenge=", res.Headers.Location.Query); + Assert.Contains("code_challenge_method=S256", res.Headers.Location.Query); + } + else + { + Assert.DoesNotContain("code_challenge=", res.Headers.Location.Query); + Assert.DoesNotContain("code_challenge_method=", res.Headers.Location.Query); + } + } + [Fact] public async Task AuthorizationRequestDoesNotIncludeTelemetryParametersWhenDisabled() { @@ -613,4 +646,4 @@ public async Task Challenge_HasOverwrittenMaxAgeParaFromBaseAuthenticationProper Assert.Contains("max_age=1234", res.Headers.Location.Query); } } -} \ No newline at end of file +} From 1431432848b2e61b8602fc778364a64c671130cb Mon Sep 17 00:00:00 2001 From: Chris R Date: Wed, 5 Jun 2019 10:49:44 -0700 Subject: [PATCH 2/8] Add IdentityServer samples --- .../Properties/launchSettings.json | 2 +- .../samples/OpenIdConnectSample/Startup.cs | 8 +- .../samples/SocialSample/Startup.cs | 269 +++++++++--------- 3 files changed, 150 insertions(+), 129 deletions(-) diff --git a/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/Properties/launchSettings.json b/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/Properties/launchSettings.json index 889821c21dc7..bc1c21c27f6b 100644 --- a/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/Properties/launchSettings.json +++ b/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/Properties/launchSettings.json @@ -15,7 +15,7 @@ "ASPNETCORE_ENVIRONMENT": "Development" } }, - "SocialSample": { + "OpenIdConnectSample": { "commandName": "Project", "launchBrowser": true, "environmentVariables": { diff --git a/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/Startup.cs b/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/Startup.cs index 484cdf8e6e58..b30f479b8cd7 100644 --- a/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/Startup.cs +++ b/src/Security/Authentication/OpenIdConnect/samples/OpenIdConnectSample/Startup.cs @@ -45,16 +45,22 @@ public void ConfigureServices(IServiceCollection services) .AddCookie() .AddOpenIdConnect(o => { + /* o.ClientId = Configuration["oidc:clientid"]; o.ClientSecret = Configuration["oidc:clientsecret"]; // for code flow o.Authority = Configuration["oidc:authority"]; + */ + // https://github.com/IdentityServer/IdentityServer4.Demo/blob/master/src/IdentityServer4Demo/Config.cs + o.ClientId = "server.hybrid"; + o.ClientSecret = "secret"; // for code flow + o.Authority = "https://demo.identityserver.io/"; o.ResponseType = OpenIdConnectResponseType.CodeIdToken; o.SaveTokens = true; o.GetClaimsFromUserInfoEndpoint = true; o.AccessDeniedPath = "/access-denied-from-remote"; - o.ClaimActions.MapAllExcept("aud", "iss", "iat", "nbf", "exp", "aio", "c_hash", "uti", "nonce"); + // o.ClaimActions.MapAllExcept("aud", "iss", "iat", "nbf", "exp", "aio", "c_hash", "uti", "nonce"); o.Events = new OpenIdConnectEvents() { diff --git a/src/Security/Authentication/samples/SocialSample/Startup.cs b/src/Security/Authentication/samples/SocialSample/Startup.cs index 77b592a8f0a7..42aade6e291c 100644 --- a/src/Security/Authentication/samples/SocialSample/Startup.cs +++ b/src/Security/Authentication/samples/SocialSample/Startup.cs @@ -50,159 +50,168 @@ public void ConfigureServices(IServiceCollection services) // https://developers.facebook.com/apps/ // https://developers.facebook.com/docs/facebook-login/manually-build-a-login-flow#login .AddFacebook(o => - { - o.AppId = Configuration["facebook:appid"]; - o.AppSecret = Configuration["facebook:appsecret"]; - o.Scope.Add("email"); - o.Fields.Add("name"); - o.Fields.Add("email"); - o.SaveTokens = true; - o.Events = new OAuthEvents() { - OnRemoteFailure = HandleOnRemoteFailure - }; - }) - // You must first create an app with Google and add its ID and Secret to your user-secrets. - // https://console.developers.google.com/project - // https://developers.google.com/identity/protocols/OAuth2WebServer - // https://developers.google.com/+/web/people/ - .AddOAuth("Google-AccessToken", "Google AccessToken only", o => - { - o.ClientId = Configuration["google:clientid"]; - o.ClientSecret = Configuration["google:clientsecret"]; - o.CallbackPath = new PathString("/signin-google-token"); - o.AuthorizationEndpoint = GoogleDefaults.AuthorizationEndpoint; - o.TokenEndpoint = GoogleDefaults.TokenEndpoint; - o.Scope.Add("openid"); - o.Scope.Add("profile"); - o.Scope.Add("email"); - o.SaveTokens = true; - o.Events = new OAuthEvents() - { - OnRemoteFailure = HandleOnRemoteFailure - }; - }) + o.AppId = Configuration["facebook:appid"]; + o.AppSecret = Configuration["facebook:appsecret"]; + o.Scope.Add("email"); + o.Fields.Add("name"); + o.Fields.Add("email"); + o.SaveTokens = true; + o.Events = new OAuthEvents() + { + OnRemoteFailure = HandleOnRemoteFailure + }; + }) // You must first create an app with Google and add its ID and Secret to your user-secrets. // https://console.developers.google.com/project // https://developers.google.com/identity/protocols/OAuth2WebServer // https://developers.google.com/+/web/people/ .AddGoogle(o => - { - o.ClientId = Configuration["google:clientid"]; - o.ClientSecret = Configuration["google:clientsecret"]; - o.AuthorizationEndpoint += "?prompt=consent"; // Hack so we always get a refresh token, it only comes on the first authorization response - o.AccessType = "offline"; - o.SaveTokens = true; - o.Events = new OAuthEvents() { - OnRemoteFailure = HandleOnRemoteFailure - }; - o.ClaimActions.MapJsonSubKey("urn:google:image", "image", "url"); - o.ClaimActions.Remove(ClaimTypes.GivenName); - }) + o.ClientId = Configuration["google:clientid"]; + o.ClientSecret = Configuration["google:clientsecret"]; + o.AuthorizationEndpoint += "?prompt=consent"; // Hack so we always get a refresh token, it only comes on the first authorization response + o.AccessType = "offline"; + o.SaveTokens = true; + o.Events = new OAuthEvents() + { + OnRemoteFailure = HandleOnRemoteFailure + }; + o.ClaimActions.MapJsonSubKey("urn:google:image", "image", "url"); + o.ClaimActions.Remove(ClaimTypes.GivenName); + }) // You must first create an app with Twitter and add its key and Secret to your user-secrets. // https://apps.twitter.com/ // https://developer.twitter.com/en/docs/basics/authentication/api-reference/access_token .AddTwitter(o => - { - o.ConsumerKey = Configuration["twitter:consumerkey"]; - o.ConsumerSecret = Configuration["twitter:consumersecret"]; - // http://stackoverflow.com/questions/22627083/can-we-get-email-id-from-twitter-oauth-api/32852370#32852370 - // http://stackoverflow.com/questions/36330675/get-users-email-from-twitter-api-for-external-login-authentication-asp-net-mvc?lq=1 - o.RetrieveUserDetails = true; - o.SaveTokens = true; - o.ClaimActions.MapJsonKey("urn:twitter:profilepicture", "profile_image_url", ClaimTypes.Uri); - o.Events = new TwitterEvents() { - OnRemoteFailure = HandleOnRemoteFailure - }; - }) + o.ConsumerKey = Configuration["twitter:consumerkey"]; + o.ConsumerSecret = Configuration["twitter:consumersecret"]; + // http://stackoverflow.com/questions/22627083/can-we-get-email-id-from-twitter-oauth-api/32852370#32852370 + // http://stackoverflow.com/questions/36330675/get-users-email-from-twitter-api-for-external-login-authentication-asp-net-mvc?lq=1 + o.RetrieveUserDetails = true; + o.SaveTokens = true; + o.ClaimActions.MapJsonKey("urn:twitter:profilepicture", "profile_image_url", ClaimTypes.Uri); + o.Events = new TwitterEvents() + { + OnRemoteFailure = HandleOnRemoteFailure + }; + }) /* Azure AD app model v2 has restrictions that prevent the use of plain HTTP for redirect URLs. - Therefore, to authenticate through microsoft accounts, tryout the sample using the following URL: + Therefore, to authenticate through microsoft accounts, try out the sample using the following URL: https://localhost:44318/ */ - // You must first create an app with Microsoft Account and add its ID and Secret to your user-secrets. - // https://apps.dev.microsoft.com/ - .AddOAuth("Microsoft-AccessToken", "Microsoft AccessToken only", o => - { - o.ClientId = Configuration["microsoftaccount:clientid"]; - o.ClientSecret = Configuration["microsoftaccount:clientsecret"]; - o.CallbackPath = new PathString("/signin-microsoft-token"); - o.AuthorizationEndpoint = MicrosoftAccountDefaults.AuthorizationEndpoint; - o.TokenEndpoint = MicrosoftAccountDefaults.TokenEndpoint; - o.Scope.Add("https://graph.microsoft.com/user.read"); - o.SaveTokens = true; - o.Events = new OAuthEvents() - { - OnRemoteFailure = HandleOnRemoteFailure - }; - }) // You must first create an app with Microsoft Account and add its ID and Secret to your user-secrets. // https://azure.microsoft.com/en-us/documentation/articles/active-directory-v2-app-registration/ + // https://apps.dev.microsoft.com/ .AddMicrosoftAccount(o => - { - o.ClientId = Configuration["microsoftaccount:clientid"]; - o.ClientSecret = Configuration["microsoftaccount:clientsecret"]; - o.SaveTokens = true; - o.Scope.Add("offline_access"); - o.Events = new OAuthEvents() { - OnRemoteFailure = HandleOnRemoteFailure - }; - }) + o.ClientId = Configuration["microsoftaccount:clientid"]; + o.ClientSecret = Configuration["microsoftaccount:clientsecret"]; + o.SaveTokens = true; + o.Scope.Add("offline_access"); + o.Events = new OAuthEvents() + { + OnRemoteFailure = HandleOnRemoteFailure + }; + }) // You must first create an app with GitHub and add its ID and Secret to your user-secrets. // https://github.com/settings/applications/ - .AddOAuth("GitHub-AccessToken", "GitHub AccessToken only", o => - { - o.ClientId = Configuration["github-token:clientid"]; - o.ClientSecret = Configuration["github-token:clientsecret"]; - o.CallbackPath = new PathString("/signin-github-token"); - o.AuthorizationEndpoint = "https://github.com/login/oauth/authorize"; - o.TokenEndpoint = "https://github.com/login/oauth/access_token"; - o.SaveTokens = true; - o.Events = new OAuthEvents() + .AddOAuth("GitHub", "Github", o => { - OnRemoteFailure = HandleOnRemoteFailure - }; - }) + o.ClientId = Configuration["github:clientid"]; + o.ClientSecret = Configuration["github:clientsecret"]; + o.CallbackPath = new PathString("/signin-github"); + o.AuthorizationEndpoint = "https://github.com/login/oauth/authorize"; + o.TokenEndpoint = "https://github.com/login/oauth/access_token"; + o.UserInformationEndpoint = "https://api.github.com/user"; + o.ClaimsIssuer = "OAuth2-Github"; + o.SaveTokens = true; + // Retrieving user information is unique to each provider. + o.ClaimActions.MapJsonKey(ClaimTypes.NameIdentifier, "id"); + o.ClaimActions.MapJsonKey(ClaimTypes.Name, "login"); + o.ClaimActions.MapJsonKey("urn:github:name", "name"); + o.ClaimActions.MapJsonKey(ClaimTypes.Email, "email", ClaimValueTypes.Email); + o.ClaimActions.MapJsonKey("urn:github:url", "url"); + o.Events = new OAuthEvents + { + OnRemoteFailure = HandleOnRemoteFailure, + OnCreatingTicket = async context => + { + // Get the GitHub user + var request = new HttpRequestMessage(HttpMethod.Get, context.Options.UserInformationEndpoint); + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", context.AccessToken); + request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json")); + + var response = await context.Backchannel.SendAsync(request, context.HttpContext.RequestAborted); + response.EnsureSuccessStatusCode(); + + using (var user = JsonDocument.Parse(await response.Content.ReadAsStringAsync())) + { + context.RunClaimActions(user.RootElement); + } + } + }; + }) // You must first create an app with GitHub and add its ID and Secret to your user-secrets. // https://github.com/settings/applications/ - .AddOAuth("GitHub", "Github", o => - { - o.ClientId = Configuration["github:clientid"]; - o.ClientSecret = Configuration["github:clientsecret"]; - o.CallbackPath = new PathString("/signin-github"); - o.AuthorizationEndpoint = "https://github.com/login/oauth/authorize"; - o.TokenEndpoint = "https://github.com/login/oauth/access_token"; - o.UserInformationEndpoint = "https://api.github.com/user"; - o.ClaimsIssuer = "OAuth2-Github"; - o.SaveTokens = true; - // Retrieving user information is unique to each provider. - o.ClaimActions.MapJsonKey(ClaimTypes.NameIdentifier, "id"); - o.ClaimActions.MapJsonKey(ClaimTypes.Name, "login"); - o.ClaimActions.MapJsonKey("urn:github:name", "name"); - o.ClaimActions.MapJsonKey(ClaimTypes.Email, "email", ClaimValueTypes.Email); - o.ClaimActions.MapJsonKey("urn:github:url", "url"); - o.Events = new OAuthEvents + .AddOAuth("GitHub-AccessToken", "GitHub AccessToken only", o => { - OnRemoteFailure = HandleOnRemoteFailure, - OnCreatingTicket = async context => + o.ClientId = Configuration["github-token:clientid"]; + o.ClientSecret = Configuration["github-token:clientsecret"]; + o.CallbackPath = new PathString("/signin-github-token"); + o.AuthorizationEndpoint = "https://github.com/login/oauth/authorize"; + o.TokenEndpoint = "https://github.com/login/oauth/access_token"; + o.SaveTokens = true; + o.Events = new OAuthEvents() { - // Get the GitHub user - var request = new HttpRequestMessage(HttpMethod.Get, context.Options.UserInformationEndpoint); - request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", context.AccessToken); - request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json")); + OnRemoteFailure = HandleOnRemoteFailure + }; + }) + // https://demo.identityserver.io/ + .AddOAuth("IdentityServer", "Identity Server", o => + { + o.ClientId = "server.code"; + o.ClientSecret = "secret"; + o.CallbackPath = new PathString("/signin-identityserver"); + o.AuthorizationEndpoint = "https://demo.identityserver.io/connect/authorize"; + o.TokenEndpoint = "https://demo.identityserver.io/connect/token"; + o.UserInformationEndpoint = "https://demo.identityserver.io/connect/userinfo"; + o.ClaimsIssuer = "IdentityServer"; + o.SaveTokens = true; + // o.UsePkse = true; + // Retrieving user information is unique to each provider. + o.ClaimActions.MapJsonKey(ClaimTypes.NameIdentifier, "sub"); + o.ClaimActions.MapJsonKey(ClaimTypes.Name, "name"); + o.ClaimActions.MapJsonKey(ClaimTypes.Email, "email", ClaimValueTypes.Email); + o.ClaimActions.MapJsonKey(ClaimTypes.GivenName, "given_name"); + o.ClaimActions.MapJsonKey(ClaimTypes.Surname, "family_name"); + o.ClaimActions.MapJsonKey("email_verified", "email_verified"); + o.ClaimActions.MapJsonKey(ClaimTypes.Uri, "website"); + o.Scope.Add("openid"); + o.Scope.Add("profile"); + o.Scope.Add("email"); + o.Scope.Add("offline_access"); + o.Events = new OAuthEvents + { + OnRemoteFailure = HandleOnRemoteFailure, + OnCreatingTicket = async context => + { + // Get the user + var request = new HttpRequestMessage(HttpMethod.Get, context.Options.UserInformationEndpoint); + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", context.AccessToken); + request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json")); - var response = await context.Backchannel.SendAsync(request, context.HttpContext.RequestAborted); - response.EnsureSuccessStatusCode(); + var response = await context.Backchannel.SendAsync(request, context.HttpContext.RequestAborted); + response.EnsureSuccessStatusCode(); - using (var user = JsonDocument.Parse(await response.Content.ReadAsStringAsync())) - { - context.RunClaimActions(user.RootElement); + using (var user = JsonDocument.Parse(await response.Content.ReadAsStringAsync())) + { + context.RunClaimActions(user.RootElement); + } } - } - }; - }); + }; + }); } private async Task HandleOnRemoteFailure(RemoteFailureContext context) @@ -210,14 +219,15 @@ private async Task HandleOnRemoteFailure(RemoteFailureContext context) context.Response.StatusCode = 500; context.Response.ContentType = "text/html"; await context.Response.WriteAsync(""); - await context.Response.WriteAsync("A remote failure has occurred: " + UrlEncoder.Default.Encode(context.Failure.Message) + "
"); + await context.Response.WriteAsync("A remote failure has occurred:
" + + context.Failure.Message.Split(Environment.NewLine).Select(s => HtmlEncoder.Default.Encode(s) + "
").Aggregate((s1, s2) => s1 + s2)); if (context.Properties != null) { await context.Response.WriteAsync("Properties:
"); foreach (var pair in context.Properties.Items) { - await context.Response.WriteAsync($"-{ UrlEncoder.Default.Encode(pair.Key)}={ UrlEncoder.Default.Encode(pair.Value)}
"); + await context.Response.WriteAsync($"-{ HtmlEncoder.Default.Encode(pair.Key)}={ HtmlEncoder.Default.Encode(pair.Value)}
"); } } @@ -295,7 +305,8 @@ public void Configure(IApplicationBuilder app) var currentAuthType = user.Identities.First().AuthenticationType; if (string.Equals(GoogleDefaults.AuthenticationScheme, currentAuthType) - || string.Equals(MicrosoftAccountDefaults.AuthenticationScheme, currentAuthType)) + || string.Equals(MicrosoftAccountDefaults.AuthenticationScheme, currentAuthType) + || string.Equals("IdentityServer", currentAuthType)) { var refreshToken = authProperties.GetTokenValue("refresh_token"); @@ -472,6 +483,10 @@ private Task GetOAuthOptionsAsync(HttpContext context, string curr { return Task.FromResult(context.RequestServices.GetRequiredService>().Get(currentAuthType)); } + else if (string.Equals("IdentityServer", currentAuthType)) + { + return Task.FromResult(context.RequestServices.GetRequiredService>().Get(currentAuthType)); + } throw new NotImplementedException(currentAuthType); } From 7840a630f225b61ce3ac3e7eb6a0c9ab06c4f36a Mon Sep 17 00:00:00 2001 From: Chris R Date: Wed, 5 Jun 2019 14:39:36 -0700 Subject: [PATCH 3/8] OAuth PKCE #7734 --- .../Core/src/RemoteAuthenticationHandler.cs | 15 ++- .../src/MicrosoftAccountOptions.cs | 1 + .../src/Events/OAuthCodeExchangeContext.cs | 39 ++++++ .../Authentication/OAuth/src/OAuthHandler.cs | 127 +++++++++++------- .../Authentication/OAuth/src/OAuthOptions.cs | 6 + .../OpenIdConnect/src/OpenIdConnectHandler.cs | 5 +- .../samples/SocialSample/Startup.cs | 2 +- .../test/MicrosoftAccountTests.cs | 73 +++++++++- 8 files changed, 211 insertions(+), 57 deletions(-) create mode 100644 src/Security/Authentication/OAuth/src/Events/OAuthCodeExchangeContext.cs diff --git a/src/Security/Authentication/Core/src/RemoteAuthenticationHandler.cs b/src/Security/Authentication/Core/src/RemoteAuthenticationHandler.cs index a8975f11a027..2e77f1609c1c 100644 --- a/src/Security/Authentication/Core/src/RemoteAuthenticationHandler.cs +++ b/src/Security/Authentication/Core/src/RemoteAuthenticationHandler.cs @@ -186,6 +186,17 @@ protected override async Task HandleAuthenticateAsync() protected override Task HandleForbiddenAsync(AuthenticationProperties properties) => Context.ForbidAsync(SignInScheme); + /// + /// Creates a 32 bit random Id and Base64Url encodes it. + /// + /// + protected virtual string GenerateUniqueId() + { + var bytes = new byte[32]; + CryptoRandom.GetBytes(bytes); + return Base64UrlTextEncoder.Encode(bytes); + } + protected virtual void GenerateCorrelationId(AuthenticationProperties properties) { if (properties == null) @@ -193,9 +204,7 @@ protected virtual void GenerateCorrelationId(AuthenticationProperties properties throw new ArgumentNullException(nameof(properties)); } - var bytes = new byte[32]; - CryptoRandom.GetBytes(bytes); - var correlationId = Base64UrlTextEncoder.Encode(bytes); + var correlationId = GenerateUniqueId(); var cookieOptions = Options.CorrelationCookie.Build(Context, Clock.UtcNow); diff --git a/src/Security/Authentication/MicrosoftAccount/src/MicrosoftAccountOptions.cs b/src/Security/Authentication/MicrosoftAccount/src/MicrosoftAccountOptions.cs index 8462913a3d93..4b76ea4016ab 100644 --- a/src/Security/Authentication/MicrosoftAccount/src/MicrosoftAccountOptions.cs +++ b/src/Security/Authentication/MicrosoftAccount/src/MicrosoftAccountOptions.cs @@ -21,6 +21,7 @@ public MicrosoftAccountOptions() AuthorizationEndpoint = MicrosoftAccountDefaults.AuthorizationEndpoint; TokenEndpoint = MicrosoftAccountDefaults.TokenEndpoint; UserInformationEndpoint = MicrosoftAccountDefaults.UserInformationEndpoint; + UsePkse = true; Scope.Add("https://graph.microsoft.com/user.read"); ClaimActions.MapJsonKey(ClaimTypes.NameIdentifier, "id"); diff --git a/src/Security/Authentication/OAuth/src/Events/OAuthCodeExchangeContext.cs b/src/Security/Authentication/OAuth/src/Events/OAuthCodeExchangeContext.cs new file mode 100644 index 000000000000..6f169be2a061 --- /dev/null +++ b/src/Security/Authentication/OAuth/src/Events/OAuthCodeExchangeContext.cs @@ -0,0 +1,39 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace Microsoft.AspNetCore.Authentication.OAuth +{ + /// + /// Contains information used to perform the code exchange. + /// + public class OAuthCodeExchangeContext + { + /// + /// Initializes a new . + /// + /// The . + /// The code returned from the authorization endpoint. + /// The redirect uri used in the authorization request. + public OAuthCodeExchangeContext(AuthenticationProperties properties, string code, string redirectUri) + { + Properties = properties; + Code = code; + RedirectUri = redirectUri; + } + + /// + /// State for the authentication flow. + /// + public AuthenticationProperties Properties { get; } + + /// + /// The code returned from the authorization endpoint. + /// + public string Code { get; } + + /// + /// The redirect uri used in the authorization request. + /// + public string RedirectUri { get; } + } +} diff --git a/src/Security/Authentication/OAuth/src/OAuthHandler.cs b/src/Security/Authentication/OAuth/src/OAuthHandler.cs index 0d2cf140663a..92dc98348d07 100644 --- a/src/Security/Authentication/OAuth/src/OAuthHandler.cs +++ b/src/Security/Authentication/OAuth/src/OAuthHandler.cs @@ -7,6 +7,7 @@ using System.Net.Http; using System.Net.Http.Headers; using System.Security.Claims; +using System.Security.Cryptography; using System.Text; using System.Text.Encodings.Web; using System.Text.Json; @@ -21,6 +22,11 @@ namespace Microsoft.AspNetCore.Authentication.OAuth { public class OAuthHandler : RemoteAuthenticationHandler where TOptions : OAuthOptions, new() { + private const string CodeVerifierKey = "code_verifier"; + private const string CodeChallengeKey = "code_challenge"; + private const string CodeChallengeMethodKey = "code_challenge_method"; + private const string CodeChallengeMethodS256 = "S256"; + protected HttpClient Backchannel => Options.Backchannel; /// @@ -99,77 +105,84 @@ protected override async Task HandleRemoteAuthenticateAsync return HandleRequestResult.Fail("Code was not found.", properties); } - using (var tokens = await ExchangeCodeAsync(code, BuildRedirectUri(Options.CallbackPath))) + var codeExchangeContext = new OAuthCodeExchangeContext(properties, code, BuildRedirectUri(Options.CallbackPath)); + using var tokens = await ExchangeCodeAsync(codeExchangeContext); + + if (tokens.Error != null) + { + return HandleRequestResult.Fail(tokens.Error, properties); + } + + if (string.IsNullOrEmpty(tokens.AccessToken)) { - if (tokens.Error != null) + return HandleRequestResult.Fail("Failed to retrieve access token.", properties); + } + + var identity = new ClaimsIdentity(ClaimsIssuer); + + if (Options.SaveTokens) + { + var authTokens = new List(); + + authTokens.Add(new AuthenticationToken { Name = "access_token", Value = tokens.AccessToken }); + if (!string.IsNullOrEmpty(tokens.RefreshToken)) { - return HandleRequestResult.Fail(tokens.Error, properties); + authTokens.Add(new AuthenticationToken { Name = "refresh_token", Value = tokens.RefreshToken }); } - if (string.IsNullOrEmpty(tokens.AccessToken)) + if (!string.IsNullOrEmpty(tokens.TokenType)) { - return HandleRequestResult.Fail("Failed to retrieve access token.", properties); + authTokens.Add(new AuthenticationToken { Name = "token_type", Value = tokens.TokenType }); } - var identity = new ClaimsIdentity(ClaimsIssuer); - - if (Options.SaveTokens) + if (!string.IsNullOrEmpty(tokens.ExpiresIn)) { - var authTokens = new List(); - - authTokens.Add(new AuthenticationToken { Name = "access_token", Value = tokens.AccessToken }); - if (!string.IsNullOrEmpty(tokens.RefreshToken)) - { - authTokens.Add(new AuthenticationToken { Name = "refresh_token", Value = tokens.RefreshToken }); - } - - if (!string.IsNullOrEmpty(tokens.TokenType)) - { - authTokens.Add(new AuthenticationToken { Name = "token_type", Value = tokens.TokenType }); - } - - if (!string.IsNullOrEmpty(tokens.ExpiresIn)) + int value; + if (int.TryParse(tokens.ExpiresIn, NumberStyles.Integer, CultureInfo.InvariantCulture, out value)) { - int value; - if (int.TryParse(tokens.ExpiresIn, NumberStyles.Integer, CultureInfo.InvariantCulture, out value)) + // https://www.w3.org/TR/xmlschema-2/#dateTime + // https://msdn.microsoft.com/en-us/library/az4se3k1(v=vs.110).aspx + var expiresAt = Clock.UtcNow + TimeSpan.FromSeconds(value); + authTokens.Add(new AuthenticationToken { - // https://www.w3.org/TR/xmlschema-2/#dateTime - // https://msdn.microsoft.com/en-us/library/az4se3k1(v=vs.110).aspx - var expiresAt = Clock.UtcNow + TimeSpan.FromSeconds(value); - authTokens.Add(new AuthenticationToken - { - Name = "expires_at", - Value = expiresAt.ToString("o", CultureInfo.InvariantCulture) - }); - } + Name = "expires_at", + Value = expiresAt.ToString("o", CultureInfo.InvariantCulture) + }); } - - properties.StoreTokens(authTokens); } - var ticket = await CreateTicketAsync(identity, properties, tokens); - if (ticket != null) - { - return HandleRequestResult.Success(ticket); - } - else - { - return HandleRequestResult.Fail("Failed to retrieve user information from remote server.", properties); - } + properties.StoreTokens(authTokens); + } + + var ticket = await CreateTicketAsync(identity, properties, tokens); + if (ticket != null) + { + return HandleRequestResult.Success(ticket); + } + else + { + return HandleRequestResult.Fail("Failed to retrieve user information from remote server.", properties); } } - protected virtual async Task ExchangeCodeAsync(string code, string redirectUri) + protected virtual async Task ExchangeCodeAsync(OAuthCodeExchangeContext context) { var tokenRequestParameters = new Dictionary() { { "client_id", Options.ClientId }, - { "redirect_uri", redirectUri }, + { "redirect_uri", context.RedirectUri }, { "client_secret", Options.ClientSecret }, - { "code", code }, + { "code", context.Code }, { "grant_type", "authorization_code" }, }; + // PKCE https://tools.ietf.org/html/rfc7636#section-4.5, see BuildChallengeUrl + if (context.Properties.Items.TryGetValue(CodeVerifierKey, out var codeVerifier)) + { + tokenRequestParameters.Add(CodeVerifierKey, codeVerifier); + context.Properties.Items.Remove(CodeVerifierKey); + } + var requestContent = new FormUrlEncodedContent(tokenRequestParameters); var requestMessage = new HttpRequestMessage(HttpMethod.Post, Options.TokenEndpoint); @@ -241,15 +254,31 @@ protected virtual string BuildChallengeUrl(AuthenticationProperties properties, var scopeParameter = properties.GetParameter>(OAuthChallengeProperties.ScopeKey); var scope = scopeParameter != null ? FormatScope(scopeParameter) : FormatScope(); - var state = Options.StateDataFormat.Protect(properties); var parameters = new Dictionary { { "client_id", Options.ClientId }, { "scope", scope }, { "response_type", "code" }, { "redirect_uri", redirectUri }, - { "state", state }, }; + + if (Options.UsePkse) + { + var codeVerifier = GenerateUniqueId(); + + // Store this for use during the code redemption. + properties.Items.Add(CodeVerifierKey, codeVerifier); + + using var sha256 = SHA256.Create(); + var challengeBytes = sha256.ComputeHash(Encoding.UTF8.GetBytes(codeVerifier)); + var codeChallenge = WebEncoders.Base64UrlEncode(challengeBytes); + + parameters[CodeChallengeKey] = codeChallenge; + parameters[CodeChallengeMethodKey] = CodeChallengeMethodS256; + } + + parameters["state"] = Options.StateDataFormat.Protect(properties); + return QueryHelpers.AddQueryString(Options.AuthorizationEndpoint, parameters); } diff --git a/src/Security/Authentication/OAuth/src/OAuthOptions.cs b/src/Security/Authentication/OAuth/src/OAuthOptions.cs index 4eacbd0b3041..a3f706b048c8 100644 --- a/src/Security/Authentication/OAuth/src/OAuthOptions.cs +++ b/src/Security/Authentication/OAuth/src/OAuthOptions.cs @@ -102,5 +102,11 @@ public override void Validate() /// Gets or sets the type used to secure data handled by the middleware. /// public ISecureDataFormat StateDataFormat { get; set; } + + /// + /// Enables or disables the use of the Proof Key for Code Exchange (PKCE) standard. See https://tools.ietf.org/html/rfc7636. + /// The default value is `false` but derived handlers should enable this if their provider supports it. + /// + public bool UsePkse { get; set; } = false; } } diff --git a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs index 2f6642a76dd0..eef83bff69fc 100644 --- a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs +++ b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs @@ -373,9 +373,7 @@ private async Task HandleChallengeAsyncInternal(AuthenticationProperties propert // https://tools.ietf.org/html/rfc7636 if (Options.UsePkse) { - var verifierBytes = new byte[32]; - CryptoRandom.GetBytes(verifierBytes); - var codeVerifier = WebEncoders.Base64UrlEncode(verifierBytes); + var codeVerifier = GenerateUniqueId(); // Store this for use during the code redemption. See RunAuthorizationCodeReceivedEventAsync. properties.Items.Add(CodeVerifierKey, codeVerifier); @@ -1123,6 +1121,7 @@ private async Task RunAuthorizationCodeReceive if (properties.Items.TryGetValue(CodeVerifierKey, out var codeVerifier)) { tokenEndpointRequest.Parameters.Add(CodeVerifierKey, codeVerifier); + properties.Items.Remove(CodeVerifierKey); } var context = new AuthorizationCodeReceivedContext(Context, Scheme, Options, properties) diff --git a/src/Security/Authentication/samples/SocialSample/Startup.cs b/src/Security/Authentication/samples/SocialSample/Startup.cs index 42aade6e291c..a19c6231f77b 100644 --- a/src/Security/Authentication/samples/SocialSample/Startup.cs +++ b/src/Security/Authentication/samples/SocialSample/Startup.cs @@ -179,7 +179,7 @@ public void ConfigureServices(IServiceCollection services) o.UserInformationEndpoint = "https://demo.identityserver.io/connect/userinfo"; o.ClaimsIssuer = "IdentityServer"; o.SaveTokens = true; - // o.UsePkse = true; + o.UsePkse = true; // Retrieving user information is unique to each provider. o.ClaimActions.MapJsonKey(ClaimTypes.NameIdentifier, "sub"); o.ClaimActions.MapJsonKey(ClaimTypes.Name, "name"); diff --git a/src/Security/Authentication/test/MicrosoftAccountTests.cs b/src/Security/Authentication/test/MicrosoftAccountTests.cs index c13f4bf51b01..650d9c294446 100644 --- a/src/Security/Authentication/test/MicrosoftAccountTests.cs +++ b/src/Security/Authentication/test/MicrosoftAccountTests.cs @@ -7,6 +7,7 @@ using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.TestHost; +using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; using System; @@ -119,6 +120,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] @@ -241,6 +244,74 @@ public async Task AuthenticatedEventCanGetRefreshToken() Assert.Equal("Test Refresh Token", transaction.FindClaimValue("RefreshToken")); } + [Fact] + public async Task PkceSentToTokenEndpoint() + { + var server = CreateServer(o => + { + o.ClientId = "Test Client Id"; + o.ClientSecret = "Test Client Secret"; + o.BackchannelHttpHandler = new TestHttpMessageHandler + { + Sender = req => + { + if (req.RequestUri.AbsoluteUri == "https://login.microsoftonline.com/common/oauth2/v2.0/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-microsoft", 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.microsoft.com/v1.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; + } + }; + }); + 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://login.microsoftonline.com/common/oauth2/v2.0/authorize", 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-microsoft?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.Microsoft.", transaction.SetCookie[0]); + Assert.StartsWith(".AspNetCore." + TestExtensions.CookieAuthenticationScheme, transaction.SetCookie[1]); + } + private static TestServer CreateServer(Action configureOptions) { var builder = new WebHostBuilder() @@ -253,7 +324,7 @@ private static TestServer CreateServer(Action configure var res = context.Response; if (req.Path == new PathString("/challenge")) { - await context.ChallengeAsync("Microsoft"); + await context.ChallengeAsync("Microsoft", new AuthenticationProperties() { RedirectUri = "/me" } ); } else if (req.Path == new PathString("/challengeWithOtherScope")) { From 34842544d8455ab637f6d001d7923387d7613bda Mon Sep 17 00:00:00 2001 From: Chris R Date: Wed, 5 Jun 2019 15:28:43 -0700 Subject: [PATCH 4/8] Ref assemblies --- ...ft.AspNetCore.Authentication.OAuth.netcoreapp3.0.cs | 10 +++++++++- ...tCore.Authentication.OpenIdConnect.netcoreapp3.0.cs | 1 + 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/Security/Authentication/OAuth/ref/Microsoft.AspNetCore.Authentication.OAuth.netcoreapp3.0.cs b/src/Security/Authentication/OAuth/ref/Microsoft.AspNetCore.Authentication.OAuth.netcoreapp3.0.cs index 2b2a35715ca1..f5786e8d2d07 100644 --- a/src/Security/Authentication/OAuth/ref/Microsoft.AspNetCore.Authentication.OAuth.netcoreapp3.0.cs +++ b/src/Security/Authentication/OAuth/ref/Microsoft.AspNetCore.Authentication.OAuth.netcoreapp3.0.cs @@ -28,6 +28,13 @@ public OAuthChallengeProperties(System.Collections.Generic.IDictionary Scope { get { throw null; } set { } } public virtual void SetScope(params string[] scopes) { } } + public partial class OAuthCodeExchangeContext + { + public OAuthCodeExchangeContext(Microsoft.AspNetCore.Authentication.AuthenticationProperties properties, string code, string redirectUri) { } + public string Code { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } + public Microsoft.AspNetCore.Authentication.AuthenticationProperties Properties { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } + public string RedirectUri { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } + } public partial class OAuthCreatingTicketContext : Microsoft.AspNetCore.Authentication.ResultContext { public OAuthCreatingTicketContext(System.Security.Claims.ClaimsPrincipal principal, Microsoft.AspNetCore.Authentication.AuthenticationProperties properties, Microsoft.AspNetCore.Http.HttpContext context, Microsoft.AspNetCore.Authentication.AuthenticationScheme scheme, Microsoft.AspNetCore.Authentication.OAuth.OAuthOptions options, System.Net.Http.HttpClient backchannel, Microsoft.AspNetCore.Authentication.OAuth.OAuthTokenResponse tokens, System.Text.Json.JsonElement user) : base (default(Microsoft.AspNetCore.Http.HttpContext), default(Microsoft.AspNetCore.Authentication.AuthenticationScheme), default(Microsoft.AspNetCore.Authentication.OAuth.OAuthOptions)) { } @@ -64,7 +71,7 @@ public OAuthEvents() { } [System.Diagnostics.DebuggerStepThroughAttribute] protected virtual System.Threading.Tasks.Task CreateTicketAsync(System.Security.Claims.ClaimsIdentity identity, Microsoft.AspNetCore.Authentication.AuthenticationProperties properties, Microsoft.AspNetCore.Authentication.OAuth.OAuthTokenResponse tokens) { throw null; } [System.Diagnostics.DebuggerStepThroughAttribute] - protected virtual System.Threading.Tasks.Task ExchangeCodeAsync(string code, string redirectUri) { throw null; } + protected virtual System.Threading.Tasks.Task ExchangeCodeAsync(Microsoft.AspNetCore.Authentication.OAuth.OAuthCodeExchangeContext context) { throw null; } protected virtual string FormatScope() { throw null; } protected virtual string FormatScope(System.Collections.Generic.IEnumerable scopes) { throw null; } [System.Diagnostics.DebuggerStepThroughAttribute] @@ -83,6 +90,7 @@ public OAuthOptions() { } public System.Collections.Generic.ICollection Scope { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } public Microsoft.AspNetCore.Authentication.ISecureDataFormat StateDataFormat { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } public string TokenEndpoint { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } + public bool UsePkse { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } public string UserInformationEndpoint { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } public override void Validate() { } } diff --git a/src/Security/Authentication/OpenIdConnect/ref/Microsoft.AspNetCore.Authentication.OpenIdConnect.netcoreapp3.0.cs b/src/Security/Authentication/OpenIdConnect/ref/Microsoft.AspNetCore.Authentication.OpenIdConnect.netcoreapp3.0.cs index 318ba7a6e9d6..e72a2fea2a07 100644 --- a/src/Security/Authentication/OpenIdConnect/ref/Microsoft.AspNetCore.Authentication.OpenIdConnect.netcoreapp3.0.cs +++ b/src/Security/Authentication/OpenIdConnect/ref/Microsoft.AspNetCore.Authentication.OpenIdConnect.netcoreapp3.0.cs @@ -135,6 +135,7 @@ public OpenIdConnectOptions() { } public Microsoft.AspNetCore.Authentication.ISecureDataFormat StateDataFormat { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } public Microsoft.AspNetCore.Authentication.ISecureDataFormat StringDataFormat { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } public Microsoft.IdentityModel.Tokens.TokenValidationParameters TokenValidationParameters { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } + public bool UsePkse { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } public bool UseTokenLifetime { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } public override void Validate() { } } From 9e42879fdab7bc7194251fc7e3e864c08fa470e3 Mon Sep 17 00:00:00 2001 From: Chris R Date: Wed, 5 Jun 2019 15:56:08 -0700 Subject: [PATCH 5/8] Typo, constants --- .../src/MicrosoftAccountOptions.cs | 2 +- ...Core.Authentication.OAuth.netcoreapp3.0.cs | 9 +++++- .../OAuth/src/OAuthConstants.cs | 31 +++++++++++++++++++ .../Authentication/OAuth/src/OAuthHandler.cs | 19 +++++------- .../Authentication/OAuth/src/OAuthOptions.cs | 2 +- ...hentication.OpenIdConnect.netcoreapp3.0.cs | 2 +- .../OpenIdConnect/src/OpenIdConnectHandler.cs | 19 +++++------- .../OpenIdConnect/src/OpenIdConnectOptions.cs | 2 +- .../samples/SocialSample/Startup.cs | 2 +- .../OpenIdConnectChallengeTests.cs | 2 +- 10 files changed, 60 insertions(+), 30 deletions(-) create mode 100644 src/Security/Authentication/OAuth/src/OAuthConstants.cs diff --git a/src/Security/Authentication/MicrosoftAccount/src/MicrosoftAccountOptions.cs b/src/Security/Authentication/MicrosoftAccount/src/MicrosoftAccountOptions.cs index 4b76ea4016ab..796cfb81421c 100644 --- a/src/Security/Authentication/MicrosoftAccount/src/MicrosoftAccountOptions.cs +++ b/src/Security/Authentication/MicrosoftAccount/src/MicrosoftAccountOptions.cs @@ -21,7 +21,7 @@ public MicrosoftAccountOptions() AuthorizationEndpoint = MicrosoftAccountDefaults.AuthorizationEndpoint; TokenEndpoint = MicrosoftAccountDefaults.TokenEndpoint; UserInformationEndpoint = MicrosoftAccountDefaults.UserInformationEndpoint; - UsePkse = true; + UsePkce = true; Scope.Add("https://graph.microsoft.com/user.read"); ClaimActions.MapJsonKey(ClaimTypes.NameIdentifier, "id"); diff --git a/src/Security/Authentication/OAuth/ref/Microsoft.AspNetCore.Authentication.OAuth.netcoreapp3.0.cs b/src/Security/Authentication/OAuth/ref/Microsoft.AspNetCore.Authentication.OAuth.netcoreapp3.0.cs index f5786e8d2d07..73d5e441da30 100644 --- a/src/Security/Authentication/OAuth/ref/Microsoft.AspNetCore.Authentication.OAuth.netcoreapp3.0.cs +++ b/src/Security/Authentication/OAuth/ref/Microsoft.AspNetCore.Authentication.OAuth.netcoreapp3.0.cs @@ -35,6 +35,13 @@ public OAuthCodeExchangeContext(Microsoft.AspNetCore.Authentication.Authenticati public Microsoft.AspNetCore.Authentication.AuthenticationProperties Properties { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } public string RedirectUri { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } } + public static partial class OAuthConstants + { + public static readonly string CodeChallengeKey; + public static readonly string CodeChallengeMethodKey; + public static readonly string CodeChallengeMethodS256; + public static readonly string CodeVerifierKey; + } public partial class OAuthCreatingTicketContext : Microsoft.AspNetCore.Authentication.ResultContext { public OAuthCreatingTicketContext(System.Security.Claims.ClaimsPrincipal principal, Microsoft.AspNetCore.Authentication.AuthenticationProperties properties, Microsoft.AspNetCore.Http.HttpContext context, Microsoft.AspNetCore.Authentication.AuthenticationScheme scheme, Microsoft.AspNetCore.Authentication.OAuth.OAuthOptions options, System.Net.Http.HttpClient backchannel, Microsoft.AspNetCore.Authentication.OAuth.OAuthTokenResponse tokens, System.Text.Json.JsonElement user) : base (default(Microsoft.AspNetCore.Http.HttpContext), default(Microsoft.AspNetCore.Authentication.AuthenticationScheme), default(Microsoft.AspNetCore.Authentication.OAuth.OAuthOptions)) { } @@ -90,7 +97,7 @@ public OAuthOptions() { } public System.Collections.Generic.ICollection Scope { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } public Microsoft.AspNetCore.Authentication.ISecureDataFormat StateDataFormat { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } public string TokenEndpoint { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } - public bool UsePkse { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } + public bool UsePkce { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } public string UserInformationEndpoint { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } public override void Validate() { } } diff --git a/src/Security/Authentication/OAuth/src/OAuthConstants.cs b/src/Security/Authentication/OAuth/src/OAuthConstants.cs new file mode 100644 index 000000000000..813dbb6e2357 --- /dev/null +++ b/src/Security/Authentication/OAuth/src/OAuthConstants.cs @@ -0,0 +1,31 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace Microsoft.AspNetCore.Authentication.OAuth +{ + /// + /// Constants used in the OAuth protocol + /// + public static class OAuthConstants + { + /// + /// code_verifier defined in https://tools.ietf.org/html/rfc7636 + /// + public static readonly string CodeVerifierKey = "code_verifier"; + + /// + /// code_challenge defined in https://tools.ietf.org/html/rfc7636 + /// + public static readonly string CodeChallengeKey = "code_challenge"; + + /// + /// code_challenge_method defined in https://tools.ietf.org/html/rfc7636 + /// + public static readonly string CodeChallengeMethodKey = "code_challenge_method"; + + /// + /// S256 defined in https://tools.ietf.org/html/rfc7636 + /// + public static readonly string CodeChallengeMethodS256 = "S256"; + } +} diff --git a/src/Security/Authentication/OAuth/src/OAuthHandler.cs b/src/Security/Authentication/OAuth/src/OAuthHandler.cs index 92dc98348d07..13b618d1da75 100644 --- a/src/Security/Authentication/OAuth/src/OAuthHandler.cs +++ b/src/Security/Authentication/OAuth/src/OAuthHandler.cs @@ -22,11 +22,6 @@ namespace Microsoft.AspNetCore.Authentication.OAuth { public class OAuthHandler : RemoteAuthenticationHandler where TOptions : OAuthOptions, new() { - private const string CodeVerifierKey = "code_verifier"; - private const string CodeChallengeKey = "code_challenge"; - private const string CodeChallengeMethodKey = "code_challenge_method"; - private const string CodeChallengeMethodS256 = "S256"; - protected HttpClient Backchannel => Options.Backchannel; /// @@ -177,10 +172,10 @@ protected virtual async Task ExchangeCodeAsync(OAuthCodeExch }; // PKCE https://tools.ietf.org/html/rfc7636#section-4.5, see BuildChallengeUrl - if (context.Properties.Items.TryGetValue(CodeVerifierKey, out var codeVerifier)) + if (context.Properties.Items.TryGetValue(OAuthConstants.CodeVerifierKey, out var codeVerifier)) { - tokenRequestParameters.Add(CodeVerifierKey, codeVerifier); - context.Properties.Items.Remove(CodeVerifierKey); + tokenRequestParameters.Add(OAuthConstants.CodeVerifierKey, codeVerifier); + context.Properties.Items.Remove(OAuthConstants.CodeVerifierKey); } var requestContent = new FormUrlEncodedContent(tokenRequestParameters); @@ -262,19 +257,19 @@ protected virtual string BuildChallengeUrl(AuthenticationProperties properties, { "redirect_uri", redirectUri }, }; - if (Options.UsePkse) + if (Options.UsePkce) { var codeVerifier = GenerateUniqueId(); // Store this for use during the code redemption. - properties.Items.Add(CodeVerifierKey, codeVerifier); + properties.Items.Add(OAuthConstants.CodeVerifierKey, codeVerifier); using var sha256 = SHA256.Create(); var challengeBytes = sha256.ComputeHash(Encoding.UTF8.GetBytes(codeVerifier)); var codeChallenge = WebEncoders.Base64UrlEncode(challengeBytes); - parameters[CodeChallengeKey] = codeChallenge; - parameters[CodeChallengeMethodKey] = CodeChallengeMethodS256; + parameters[OAuthConstants.CodeChallengeKey] = codeChallenge; + parameters[OAuthConstants.CodeChallengeMethodKey] = OAuthConstants.CodeChallengeMethodS256; } parameters["state"] = Options.StateDataFormat.Protect(properties); diff --git a/src/Security/Authentication/OAuth/src/OAuthOptions.cs b/src/Security/Authentication/OAuth/src/OAuthOptions.cs index a3f706b048c8..821425b27452 100644 --- a/src/Security/Authentication/OAuth/src/OAuthOptions.cs +++ b/src/Security/Authentication/OAuth/src/OAuthOptions.cs @@ -107,6 +107,6 @@ public override void Validate() /// Enables or disables the use of the Proof Key for Code Exchange (PKCE) standard. See https://tools.ietf.org/html/rfc7636. /// The default value is `false` but derived handlers should enable this if their provider supports it. /// - public bool UsePkse { get; set; } = false; + public bool UsePkce { get; set; } = false; } } diff --git a/src/Security/Authentication/OpenIdConnect/ref/Microsoft.AspNetCore.Authentication.OpenIdConnect.netcoreapp3.0.cs b/src/Security/Authentication/OpenIdConnect/ref/Microsoft.AspNetCore.Authentication.OpenIdConnect.netcoreapp3.0.cs index e72a2fea2a07..998956d2ed8a 100644 --- a/src/Security/Authentication/OpenIdConnect/ref/Microsoft.AspNetCore.Authentication.OpenIdConnect.netcoreapp3.0.cs +++ b/src/Security/Authentication/OpenIdConnect/ref/Microsoft.AspNetCore.Authentication.OpenIdConnect.netcoreapp3.0.cs @@ -135,7 +135,7 @@ public OpenIdConnectOptions() { } public Microsoft.AspNetCore.Authentication.ISecureDataFormat StateDataFormat { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } public Microsoft.AspNetCore.Authentication.ISecureDataFormat StringDataFormat { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } public Microsoft.IdentityModel.Tokens.TokenValidationParameters TokenValidationParameters { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } - public bool UsePkse { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } + public bool UsePkce { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } public bool UseTokenLifetime { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } public override void Validate() { } } diff --git a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs index eef83bff69fc..9013aa793087 100644 --- a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs +++ b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs @@ -14,6 +14,7 @@ using System.Text.Encodings.Web; using System.Text.Json; using System.Threading.Tasks; +using Microsoft.AspNetCore.Authentication.OAuth; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.Logging; @@ -32,10 +33,6 @@ public class OpenIdConnectHandler : RemoteAuthenticationHandler RunAuthorizationCodeReceive }; // PKCE https://tools.ietf.org/html/rfc7636#section-4.5, see HandleChallengeAsyncInternal - if (properties.Items.TryGetValue(CodeVerifierKey, out var codeVerifier)) + if (properties.Items.TryGetValue(OAuthConstants.CodeVerifierKey, out var codeVerifier)) { - tokenEndpointRequest.Parameters.Add(CodeVerifierKey, codeVerifier); - properties.Items.Remove(CodeVerifierKey); + tokenEndpointRequest.Parameters.Add(OAuthConstants.CodeVerifierKey, codeVerifier); + properties.Items.Remove(OAuthConstants.CodeVerifierKey); } var context = new AuthorizationCodeReceivedContext(Context, Scheme, Options, properties) diff --git a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs index 96389c36d088..037e23b9bf2c 100644 --- a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs +++ b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs @@ -304,7 +304,7 @@ public CookieBuilder NonceCookie /// See https://tools.ietf.org/html/rfc7636. /// The default value is `true`. /// - public bool UsePkse { get; set; } = true; + public bool UsePkce { get; set; } = true; private class OpenIdConnectNonceCookieBuilder : RequestPathBaseCookieBuilder { diff --git a/src/Security/Authentication/samples/SocialSample/Startup.cs b/src/Security/Authentication/samples/SocialSample/Startup.cs index a19c6231f77b..2c85c22c80cf 100644 --- a/src/Security/Authentication/samples/SocialSample/Startup.cs +++ b/src/Security/Authentication/samples/SocialSample/Startup.cs @@ -179,7 +179,7 @@ public void ConfigureServices(IServiceCollection services) o.UserInformationEndpoint = "https://demo.identityserver.io/connect/userinfo"; o.ClaimsIssuer = "IdentityServer"; o.SaveTokens = true; - o.UsePkse = true; + o.UsePkce = true; // Retrieving user information is unique to each provider. o.ClaimActions.MapJsonKey(ClaimTypes.NameIdentifier, "sub"); o.ClaimActions.MapJsonKey(ClaimTypes.Name, "name"); diff --git a/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs b/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs index f02d5d352bb4..915b388e4ded 100644 --- a/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs +++ b/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs @@ -59,7 +59,7 @@ public async Task ChallengeIncludesPkceIfRequested(bool include) opt.Authority = TestServerBuilder.DefaultAuthority; opt.AuthenticationMethod = OpenIdConnectRedirectBehavior.RedirectGet; opt.ClientId = "Test Id"; - opt.UsePkse = include; + opt.UsePkce = include; }); var server = settings.CreateTestServer(); From 7864b08ad4e0410435d42252c38465f310fdd0fa Mon Sep 17 00:00:00 2001 From: Chris R Date: Wed, 5 Jun 2019 16:23:24 -0700 Subject: [PATCH 6/8] Code only --- .../OpenIdConnect/src/OpenIdConnectHandler.cs | 2 +- .../OpenIdConnect/src/OpenIdConnectOptions.cs | 1 + .../OpenIdConnectChallengeTests.cs | 28 +++++++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs index 9013aa793087..b593214d491d 100644 --- a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs +++ b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs @@ -368,7 +368,7 @@ private async Task HandleChallengeAsyncInternal(AuthenticationProperties propert }; // https://tools.ietf.org/html/rfc7636 - if (Options.UsePkce) + if (Options.UsePkce && Options.ResponseType == OpenIdConnectResponseType.Code) { var codeVerifier = GenerateUniqueId(); diff --git a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs index 037e23b9bf2c..f274760d995d 100644 --- a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs +++ b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs @@ -301,6 +301,7 @@ public CookieBuilder NonceCookie /// /// Enables or disables the use of the Proof Key for Code Exchange (PKCE) standard. + /// This only applies when the is set to . /// See https://tools.ietf.org/html/rfc7636. /// The default value is `true`. /// diff --git a/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs b/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs index 915b388e4ded..d6836591182b 100644 --- a/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs +++ b/src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs @@ -58,6 +58,7 @@ public async Task ChallengeIncludesPkceIfRequested(bool include) { opt.Authority = TestServerBuilder.DefaultAuthority; opt.AuthenticationMethod = OpenIdConnectRedirectBehavior.RedirectGet; + opt.ResponseType = OpenIdConnectResponseType.Code; opt.ClientId = "Test Id"; opt.UsePkce = include; }); @@ -81,6 +82,33 @@ public async Task ChallengeIncludesPkceIfRequested(bool include) } } + [Theory] + [InlineData(OpenIdConnectResponseType.Token)] + [InlineData(OpenIdConnectResponseType.IdToken)] + [InlineData(OpenIdConnectResponseType.CodeIdToken)] + public async Task ChallengeDoesNotIncludePkceForOtherResponseTypes(string responseType) + { + var settings = new TestSettings( + opt => + { + opt.Authority = TestServerBuilder.DefaultAuthority; + opt.AuthenticationMethod = OpenIdConnectRedirectBehavior.RedirectGet; + opt.ResponseType = responseType; + opt.ClientId = "Test Id"; + opt.UsePkce = true; + }); + + var server = settings.CreateTestServer(); + var transaction = await server.SendAsync(ChallengeEndpoint); + + var res = transaction.Response; + Assert.Equal(HttpStatusCode.Redirect, res.StatusCode); + Assert.NotNull(res.Headers.Location); + + Assert.DoesNotContain("code_challenge=", res.Headers.Location.Query); + Assert.DoesNotContain("code_challenge_method=", res.Headers.Location.Query); + } + [Fact] public async Task AuthorizationRequestDoesNotIncludeTelemetryParametersWhenDisabled() { From 3548190a05756800204129d9b4c77bac0f578d9d Mon Sep 17 00:00:00 2001 From: Chris R Date: Thu, 6 Jun 2019 09:10:27 -0700 Subject: [PATCH 7/8] Refs ~!@@~~ --- .../ref/Microsoft.AspNetCore.Authentication.netcoreapp3.0.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Security/Authentication/Core/ref/Microsoft.AspNetCore.Authentication.netcoreapp3.0.cs b/src/Security/Authentication/Core/ref/Microsoft.AspNetCore.Authentication.netcoreapp3.0.cs index 6a2cfd1a9e92..66592a76d1f4 100644 --- a/src/Security/Authentication/Core/ref/Microsoft.AspNetCore.Authentication.netcoreapp3.0.cs +++ b/src/Security/Authentication/Core/ref/Microsoft.AspNetCore.Authentication.netcoreapp3.0.cs @@ -203,6 +203,7 @@ public RemoteAuthenticationEvents() { } protected string SignInScheme { get { throw null; } } protected override System.Threading.Tasks.Task CreateEventsAsync() { throw null; } protected virtual void GenerateCorrelationId(Microsoft.AspNetCore.Authentication.AuthenticationProperties properties) { } + protected virtual string GenerateUniqueId() { throw null; } [System.Diagnostics.DebuggerStepThroughAttribute] protected virtual System.Threading.Tasks.Task HandleAccessDeniedErrorAsync(Microsoft.AspNetCore.Authentication.AuthenticationProperties properties) { throw null; } [System.Diagnostics.DebuggerStepThroughAttribute] From 83289f0043ded9fc0a8ff8cfc4993c2c8c2b6500 Mon Sep 17 00:00:00 2001 From: Chris R Date: Fri, 7 Jun 2019 09:33:55 -0700 Subject: [PATCH 8/8] Remove GenerateUniqueId. Move OAuthCodeExchangeContext. --- ...oft.AspNetCore.Authentication.netcoreapp3.0.cs | 1 - .../Core/src/RemoteAuthenticationHandler.cs | 15 +++------------ .../src/{Events => }/OAuthCodeExchangeContext.cs | 2 +- .../Authentication/OAuth/src/OAuthHandler.cs | 5 ++++- .../OpenIdConnect/src/OpenIdConnectHandler.cs | 4 +++- 5 files changed, 11 insertions(+), 16 deletions(-) rename src/Security/Authentication/OAuth/src/{Events => }/OAuthCodeExchangeContext.cs (95%) diff --git a/src/Security/Authentication/Core/ref/Microsoft.AspNetCore.Authentication.netcoreapp3.0.cs b/src/Security/Authentication/Core/ref/Microsoft.AspNetCore.Authentication.netcoreapp3.0.cs index 66592a76d1f4..6a2cfd1a9e92 100644 --- a/src/Security/Authentication/Core/ref/Microsoft.AspNetCore.Authentication.netcoreapp3.0.cs +++ b/src/Security/Authentication/Core/ref/Microsoft.AspNetCore.Authentication.netcoreapp3.0.cs @@ -203,7 +203,6 @@ public RemoteAuthenticationEvents() { } protected string SignInScheme { get { throw null; } } protected override System.Threading.Tasks.Task CreateEventsAsync() { throw null; } protected virtual void GenerateCorrelationId(Microsoft.AspNetCore.Authentication.AuthenticationProperties properties) { } - protected virtual string GenerateUniqueId() { throw null; } [System.Diagnostics.DebuggerStepThroughAttribute] protected virtual System.Threading.Tasks.Task HandleAccessDeniedErrorAsync(Microsoft.AspNetCore.Authentication.AuthenticationProperties properties) { throw null; } [System.Diagnostics.DebuggerStepThroughAttribute] diff --git a/src/Security/Authentication/Core/src/RemoteAuthenticationHandler.cs b/src/Security/Authentication/Core/src/RemoteAuthenticationHandler.cs index 2e77f1609c1c..a8975f11a027 100644 --- a/src/Security/Authentication/Core/src/RemoteAuthenticationHandler.cs +++ b/src/Security/Authentication/Core/src/RemoteAuthenticationHandler.cs @@ -186,17 +186,6 @@ protected override async Task HandleAuthenticateAsync() protected override Task HandleForbiddenAsync(AuthenticationProperties properties) => Context.ForbidAsync(SignInScheme); - /// - /// Creates a 32 bit random Id and Base64Url encodes it. - /// - /// - protected virtual string GenerateUniqueId() - { - var bytes = new byte[32]; - CryptoRandom.GetBytes(bytes); - return Base64UrlTextEncoder.Encode(bytes); - } - protected virtual void GenerateCorrelationId(AuthenticationProperties properties) { if (properties == null) @@ -204,7 +193,9 @@ protected virtual void GenerateCorrelationId(AuthenticationProperties properties throw new ArgumentNullException(nameof(properties)); } - var correlationId = GenerateUniqueId(); + var bytes = new byte[32]; + CryptoRandom.GetBytes(bytes); + var correlationId = Base64UrlTextEncoder.Encode(bytes); var cookieOptions = Options.CorrelationCookie.Build(Context, Clock.UtcNow); diff --git a/src/Security/Authentication/OAuth/src/Events/OAuthCodeExchangeContext.cs b/src/Security/Authentication/OAuth/src/OAuthCodeExchangeContext.cs similarity index 95% rename from src/Security/Authentication/OAuth/src/Events/OAuthCodeExchangeContext.cs rename to src/Security/Authentication/OAuth/src/OAuthCodeExchangeContext.cs index 6f169be2a061..a8dbaf3e2a43 100644 --- a/src/Security/Authentication/OAuth/src/Events/OAuthCodeExchangeContext.cs +++ b/src/Security/Authentication/OAuth/src/OAuthCodeExchangeContext.cs @@ -9,7 +9,7 @@ namespace Microsoft.AspNetCore.Authentication.OAuth public class OAuthCodeExchangeContext { /// - /// Initializes a new . + /// Initializes a new . /// /// The . /// The code returned from the authorization endpoint. diff --git a/src/Security/Authentication/OAuth/src/OAuthHandler.cs b/src/Security/Authentication/OAuth/src/OAuthHandler.cs index 13b618d1da75..0424020db214 100644 --- a/src/Security/Authentication/OAuth/src/OAuthHandler.cs +++ b/src/Security/Authentication/OAuth/src/OAuthHandler.cs @@ -22,6 +22,7 @@ namespace Microsoft.AspNetCore.Authentication.OAuth { public class OAuthHandler : RemoteAuthenticationHandler where TOptions : OAuthOptions, new() { + private static readonly RandomNumberGenerator CryptoRandom = RandomNumberGenerator.Create(); protected HttpClient Backchannel => Options.Backchannel; /// @@ -259,7 +260,9 @@ protected virtual string BuildChallengeUrl(AuthenticationProperties properties, if (Options.UsePkce) { - var codeVerifier = GenerateUniqueId(); + var bytes = new byte[32]; + CryptoRandom.GetBytes(bytes); + var codeVerifier = Base64UrlTextEncoder.Encode(bytes); // Store this for use during the code redemption. properties.Items.Add(OAuthConstants.CodeVerifierKey, codeVerifier); diff --git a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs index b593214d491d..4a9a259398c3 100644 --- a/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs +++ b/src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs @@ -370,7 +370,9 @@ private async Task HandleChallengeAsyncInternal(AuthenticationProperties propert // https://tools.ietf.org/html/rfc7636 if (Options.UsePkce && Options.ResponseType == OpenIdConnectResponseType.Code) { - var codeVerifier = GenerateUniqueId(); + var bytes = new byte[32]; + CryptoRandom.GetBytes(bytes); + var codeVerifier = Base64UrlTextEncoder.Encode(bytes); // Store this for use during the code redemption. See RunAuthorizationCodeReceivedEventAsync. properties.Items.Add(OAuthConstants.CodeVerifierKey, codeVerifier);