From 276a9b8a7d40659e83544aedd6ef9f2203cbeae1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Chalet?= Date: Wed, 22 Jan 2020 02:27:15 +0100 Subject: [PATCH] Update OpenIddict.Validation.SystemNetHttp to use ReadAsStringAsync() and rework the samples --- samples/Mvc.Client/Program.cs | 13 +- samples/Mvc.Server/Program.cs | 13 +- samples/Mvc.Server/Startup.cs | 107 +-------------- samples/Mvc.Server/Worker.cs | 128 ++++++++++++++++++ .../OpenIddictServerHandlers.Exchange.cs | 30 ++-- ...enIddictValidationSystemNetHttpHandlers.cs | 13 +- .../OpenIddictServerIntegrationTestClient.cs | 62 +++++---- 7 files changed, 202 insertions(+), 164 deletions(-) create mode 100644 samples/Mvc.Server/Worker.cs diff --git a/samples/Mvc.Client/Program.cs b/samples/Mvc.Client/Program.cs index a06f691c..a15fd7dc 100644 --- a/samples/Mvc.Client/Program.cs +++ b/samples/Mvc.Client/Program.cs @@ -1,16 +1,15 @@ -using Microsoft.AspNetCore; -using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Hosting; +using Microsoft.Extensions.Hosting; namespace Mvc.Client { public static class Program { public static void Main(string[] args) => - BuildWebHost(args).Run(); + CreateHostBuilder(args).Build().Run(); - public static IWebHost BuildWebHost(string[] args) => - WebHost.CreateDefaultBuilder(args) - .UseStartup() - .Build(); + public static IHostBuilder CreateHostBuilder(string[] args) => + Host.CreateDefaultBuilder(args) + .ConfigureWebHostDefaults(builder => builder.UseStartup()); } } diff --git a/samples/Mvc.Server/Program.cs b/samples/Mvc.Server/Program.cs index 70e9ba9f..16b0ecfa 100644 --- a/samples/Mvc.Server/Program.cs +++ b/samples/Mvc.Server/Program.cs @@ -1,16 +1,15 @@ -using Microsoft.AspNetCore; -using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Hosting; +using Microsoft.Extensions.Hosting; namespace Mvc.Server { public static class Program { public static void Main(string[] args) => - BuildWebHost(args).Run(); + CreateHostBuilder(args).Build().Run(); - public static IWebHost BuildWebHost(string[] args) => - WebHost.CreateDefaultBuilder(args) - .UseStartup() - .Build(); + public static IHostBuilder CreateHostBuilder(string[] args) => + Host.CreateDefaultBuilder(args) + .ConfigureWebHostDefaults(builder => builder.UseStartup()); } } diff --git a/samples/Mvc.Server/Startup.cs b/samples/Mvc.Server/Startup.cs index f1526409..664310c7 100644 --- a/samples/Mvc.Server/Startup.cs +++ b/samples/Mvc.Server/Startup.cs @@ -1,5 +1,3 @@ -using System; -using System.Threading.Tasks; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Identity; @@ -8,9 +6,6 @@ using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Mvc.Server.Models; using Mvc.Server.Services; -using OpenIddict.Abstractions; -using OpenIddict.Core; -using OpenIddict.EntityFrameworkCore.Models; using static OpenIddict.Abstractions.OpenIddictConstants; namespace Mvc.Server @@ -137,6 +132,10 @@ namespace Mvc.Server services.AddTransient(); services.AddTransient(); + + // Register the worker responsible of seeding the database with the sample clients. + // Note: in a real world application, this step should be part of a setup script. + services.AddHostedService(); } public void Configure(IApplicationBuilder app) @@ -166,104 +165,6 @@ namespace Mvc.Server app.UseEndpoints(options => options.MapControllerRoute( name: "default", pattern: "{controller=Home}/{action=Index}/{id?}")); - - // Seed the database with the sample applications. - // Note: in a real world application, this step should be part of a setup script. - InitializeAsync(app.ApplicationServices).GetAwaiter().GetResult(); - } - - private async Task InitializeAsync(IServiceProvider services) - { - // Create a new service scope to ensure the database context is correctly disposed when this methods returns. - using var scope = services.GetRequiredService().CreateScope(); - - var context = scope.ServiceProvider.GetRequiredService(); - await context.Database.EnsureCreatedAsync(); - - await RegisterApplicationsAsync(scope.ServiceProvider); - await RegisterScopesAsync(scope.ServiceProvider); - - static async Task RegisterApplicationsAsync(IServiceProvider provider) - { - var manager = provider.GetRequiredService>(); - - if (await manager.FindByClientIdAsync("mvc") == null) - { - await manager.CreateAsync(new OpenIddictApplicationDescriptor - { - ClientId = "mvc", - ClientSecret = "901564A5-E7FE-42CB-B10D-61EF6A8F3654", - ConsentType = ConsentTypes.Explicit, - DisplayName = "MVC client application", - PostLogoutRedirectUris = { new Uri("http://localhost:53507/signout-callback-oidc") }, - RedirectUris = { new Uri("http://localhost:53507/signin-oidc") }, - Permissions = - { - Permissions.Endpoints.Authorization, - Permissions.Endpoints.Logout, - Permissions.Endpoints.Token, - Permissions.GrantTypes.AuthorizationCode, - Permissions.GrantTypes.RefreshToken, - Permissions.Scopes.Email, - Permissions.Scopes.Profile, - Permissions.Scopes.Roles, - Permissions.Prefixes.Scope + "demo_api" - }, - Requirements = - { - Requirements.Features.ProofKeyForCodeExchange - } - }); - } - - // To test this sample with Postman, use the following settings: - // - // * Authorization URL: http://localhost:54540/connect/authorize - // * Access token URL: http://localhost:54540/connect/token - // * Client ID: postman - // * Client secret: [blank] (not used with public clients) - // * Scope: openid email profile roles - // * Grant type: authorization code - // * Request access token locally: yes - if (await manager.FindByClientIdAsync("postman") == null) - { - await manager.CreateAsync(new OpenIddictApplicationDescriptor - { - ClientId = "postman", - ConsentType = ConsentTypes.Systematic, - DisplayName = "Postman", - RedirectUris = { new Uri("urn:postman") }, - Permissions = - { - Permissions.Endpoints.Authorization, - Permissions.Endpoints.Device, - Permissions.Endpoints.Token, - Permissions.GrantTypes.AuthorizationCode, - Permissions.GrantTypes.DeviceCode, - Permissions.GrantTypes.Password, - Permissions.GrantTypes.RefreshToken, - Permissions.Scopes.Email, - Permissions.Scopes.Profile, - Permissions.Scopes.Roles - } - }); - } - } - - static async Task RegisterScopesAsync(IServiceProvider provider) - { - var manager = provider.GetRequiredService>(); - - if (await manager.FindByNameAsync("demo_api") == null) - { - await manager.CreateAsync(new OpenIddictScopeDescriptor - { - DisplayName = "Demo API access", - Name = "demo_api", - Resources = { "resource_server" } - }); - } - } } } } diff --git a/samples/Mvc.Server/Worker.cs b/samples/Mvc.Server/Worker.cs new file mode 100644 index 00000000..cafedbab --- /dev/null +++ b/samples/Mvc.Server/Worker.cs @@ -0,0 +1,128 @@ +using System; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Mvc.Server.Models; +using OpenIddict.Abstractions; +using OpenIddict.Core; +using OpenIddict.EntityFrameworkCore.Models; +using static OpenIddict.Abstractions.OpenIddictConstants; + +namespace Mvc.Server +{ + public class Worker : IHostedService + { + private readonly IServiceProvider _serviceProvider; + + public Worker(IServiceProvider serviceScopeFactory) + => _serviceProvider = serviceScopeFactory; + + public async Task StartAsync(CancellationToken cancellationToken) + { + using var scope = _serviceProvider.CreateScope(); + + var context = scope.ServiceProvider.GetRequiredService(); + await context.Database.EnsureCreatedAsync(); + + await RegisterApplicationsAsync(scope.ServiceProvider); + await RegisterScopesAsync(scope.ServiceProvider); + + static async Task RegisterApplicationsAsync(IServiceProvider provider) + { + var manager = provider.GetRequiredService>(); + + if (await manager.FindByClientIdAsync("mvc") == null) + { + await manager.CreateAsync(new OpenIddictApplicationDescriptor + { + ClientId = "mvc", + ClientSecret = "901564A5-E7FE-42CB-B10D-61EF6A8F3654", + ConsentType = ConsentTypes.Explicit, + DisplayName = "MVC client application", + PostLogoutRedirectUris = + { + new Uri("http://localhost:53507/signout-callback-oidc") + }, + RedirectUris = + { + new Uri("http://localhost:53507/signin-oidc") + }, + Permissions = + { + Permissions.Endpoints.Authorization, + Permissions.Endpoints.Logout, + Permissions.Endpoints.Token, + Permissions.GrantTypes.AuthorizationCode, + Permissions.GrantTypes.RefreshToken, + Permissions.Scopes.Email, + Permissions.Scopes.Profile, + Permissions.Scopes.Roles, + Permissions.Prefixes.Scope + "demo_api" + }, + Requirements = + { + Requirements.Features.ProofKeyForCodeExchange + } + }); + } + + // To test this sample with Postman, use the following settings: + // + // * Authorization URL: http://localhost:54540/connect/authorize + // * Access token URL: http://localhost:54540/connect/token + // * Client ID: postman + // * Client secret: [blank] (not used with public clients) + // * Scope: openid email profile roles + // * Grant type: authorization code + // * Request access token locally: yes + if (await manager.FindByClientIdAsync("postman") == null) + { + await manager.CreateAsync(new OpenIddictApplicationDescriptor + { + ClientId = "postman", + ConsentType = ConsentTypes.Systematic, + DisplayName = "Postman", + RedirectUris = + { + new Uri("urn:postman") + }, + Permissions = + { + Permissions.Endpoints.Authorization, + Permissions.Endpoints.Device, + Permissions.Endpoints.Token, + Permissions.GrantTypes.AuthorizationCode, + Permissions.GrantTypes.DeviceCode, + Permissions.GrantTypes.Password, + Permissions.GrantTypes.RefreshToken, + Permissions.Scopes.Email, + Permissions.Scopes.Profile, + Permissions.Scopes.Roles + } + }); + } + } + + static async Task RegisterScopesAsync(IServiceProvider provider) + { + var manager = provider.GetRequiredService>(); + + if (await manager.FindByNameAsync("demo_api") == null) + { + await manager.CreateAsync(new OpenIddictScopeDescriptor + { + DisplayName = "Demo API access", + Name = "demo_api", + Resources = + { + "resource_server" + } + }); + } + } + } + + public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask; + } +} diff --git a/src/OpenIddict.Server/OpenIddictServerHandlers.Exchange.cs b/src/OpenIddict.Server/OpenIddictServerHandlers.Exchange.cs index a0af0b54..5d1a69c3 100644 --- a/src/OpenIddict.Server/OpenIddictServerHandlers.Exchange.cs +++ b/src/OpenIddict.Server/OpenIddictServerHandlers.Exchange.cs @@ -1611,26 +1611,24 @@ namespace OpenIddict.Server // is active even if the degraded mode is enabled and ensures that a code_verifier is sent if a // code_challenge was stored in the authorization code when the authorization request was handled. - // Validate that the token request did not include the code_verifier parameter if the - // authorization request was not provided with both code_challenge and code_challenge_method. var challenge = context.Principal.GetClaim(Claims.Private.CodeChallenge); - if (!string.IsNullOrEmpty(context.Request.CodeVerifier) && string.IsNullOrEmpty(challenge)) + if (string.IsNullOrEmpty(challenge)) { - context.Logger.LogError("The token request was rejected because a 'code_verifier' " + - "parameter was presented but 'code_challenge' and/or 'code_challenge_method' " + - "was not a part of the authorization request."); + // Validate that the token request does not include a code_verifier parameter + // when code_challenge private claim was attached to the authorization code. + if (!string.IsNullOrEmpty(context.Request.CodeVerifier)) + { + context.Logger.LogError("The token request was rejected because a 'code_verifier' parameter " + + "was presented with an authorization code to which no code challenge " + + "was attached when processing the initial authorization request."); - context.Reject( - error: Errors.InvalidRequest, - description: "The 'code_verifier' parameter is uncalled for in this request."); + context.Reject( + error: Errors.InvalidRequest, + description: "The 'code_verifier' parameter is uncalled for in this request."); - return default; - } + return default; + } - // If a code challenge was initially sent in the authorization request and associated with the - // code, validate the code verifier to ensure the token request is sent by a legit caller. - if (string.IsNullOrEmpty(challenge)) - { return default; } @@ -1676,7 +1674,7 @@ namespace OpenIddict.Server // Compare the verifier and the code challenge: if the two don't match, return an error. // Note: to prevent timing attacks, a time-constant comparer is always used. - if (!FixedTimeEquals(data, Encoding.UTF8.GetBytes(challenge))) + if (!FixedTimeEquals(data, Encoding.ASCII.GetBytes(challenge))) { context.Logger.LogError("The token request was rejected because the 'code_verifier' was invalid."); diff --git a/src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpHandlers.cs b/src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpHandlers.cs index cb4fbca6..b6b2e778 100644 --- a/src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpHandlers.cs +++ b/src/OpenIddict.Validation.SystemNetHttp/OpenIddictValidationSystemNetHttpHandlers.cs @@ -169,6 +169,7 @@ namespace OpenIddict.Validation.SystemNetHttp { using var request = new HttpRequestMessage(HttpMethod.Get, address); request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json")); + request.Headers.AcceptCharset.Add(new StringWithQualityHeaderValue("utf-8")); using var response = await client.SendAsync(request, HttpCompletionOption.ResponseContentRead); if (!response.IsSuccessStatusCode) @@ -181,8 +182,8 @@ namespace OpenIddict.Validation.SystemNetHttp /* Body: */ await response.Content.ReadAsStringAsync())); } - var media = response.Content?.Headers.ContentType?.MediaType; - if (!string.Equals(media, "application/json", StringComparison.OrdinalIgnoreCase)) + var type = response.Content?.Headers.ContentType?.MediaType; + if (!string.Equals(type, "application/json", StringComparison.OrdinalIgnoreCase)) { throw new InvalidOperationException(string.Format(CultureInfo.InvariantCulture, "The OAuth 2.0/OpenID Connect discovery failed because an invalid content type was received:" + @@ -192,8 +193,12 @@ namespace OpenIddict.Validation.SystemNetHttp /* Body: */ await response.Content.ReadAsStringAsync())); } - using var stream = await response.Content.ReadAsStreamAsync(); - return await JsonSerializer.DeserializeAsync(stream); + // Note: ReadAsStreamAsync() is deliberately not used here, as we can't guarantee that + // the validation handler will always be used with OAuth 2.0 servers returning UTF-8 + // responses (which is not required by the OAuth 2.0/OpenID Connect discovery specs). + // Unlike ReadAsStreamAsync(), ReadAsStringAsync() will use the response charset + // to determine whether the payload is UTF-8-encoded and transcode it if necessary. + return JsonSerializer.Deserialize(await response.Content.ReadAsStringAsync()); } } } diff --git a/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTestClient.cs b/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTestClient.cs index d0665e7c..6d8d96f0 100644 --- a/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTestClient.cs +++ b/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTestClient.cs @@ -380,6 +380,9 @@ namespace OpenIddict.Server.FunctionalTests else if (string.Equals(message.Content?.Headers?.ContentType?.MediaType, "application/json", StringComparison.OrdinalIgnoreCase)) { + // Note: this test client is only used with OpenIddict's ASP.NET Core or OWIN hosts, + // that always return their HTTP responses encoded using UTF-8. As such, the stream + // returned by ReadAsStreamAsync() is always assumed to contain UTF-8 encoded payloads. using var stream = await message.Content.ReadAsStreamAsync(); return await JsonSerializer.DeserializeAsync(stream); @@ -387,6 +390,9 @@ namespace OpenIddict.Server.FunctionalTests else if (string.Equals(message.Content?.Headers?.ContentType?.MediaType, "text/html", StringComparison.OrdinalIgnoreCase)) { + // Note: this test client is only used with OpenIddict's ASP.NET Core or OWIN hosts, + // that always return their HTTP responses encoded using UTF-8. As such, the stream + // returned by ReadAsStreamAsync() is always assumed to contain UTF-8 encoded payloads. using var stream = await message.Content.ReadAsStreamAsync(); using var document = await HtmlParser.ParseDocumentAsync(stream); @@ -417,39 +423,41 @@ namespace OpenIddict.Server.FunctionalTests else if (string.Equals(message.Content?.Headers?.ContentType?.MediaType, "text/plain", StringComparison.OrdinalIgnoreCase)) { - using (var stream = await message.Content.ReadAsStreamAsync()) - using (var reader = new StreamReader(stream)) - { - // Note: a dictionary is deliberately not used here to allow multiple parameters with the - // same name to be retrieved. While initially not allowed by the core OAuth2 specification, - // this is required for derived drafts like the OAuth2 token exchange specification. - var parameters = new List>(); - - for (var line = await reader.ReadLineAsync(); line != null; line = await reader.ReadLineAsync()) - { - var index = line.IndexOf(':'); - if (index == -1) - { - continue; - } + // Note: this test client is only used with OpenIddict's ASP.NET Core or OWIN hosts, + // that always return their HTTP responses encoded using UTF-8. As such, the stream + // returned by ReadAsStreamAsync() is always assumed to contain UTF-8 encoded payloads. + using var stream = await message.Content.ReadAsStreamAsync(); + using var reader = new StreamReader(stream); - var name = line.Substring(0, index); - if (string.IsNullOrEmpty(name)) - { - continue; - } + // Note: a dictionary is deliberately not used here to allow multiple parameters with the + // same name to be retrieved. While initially not allowed by the core OAuth2 specification, + // this is required for derived drafts like the OAuth2 token exchange specification. + var parameters = new List>(); - var value = line.Substring(index + 1); + for (var line = await reader.ReadLineAsync(); line != null; line = await reader.ReadLineAsync()) + { + var index = line.IndexOf(':'); + if (index == -1) + { + continue; + } - parameters.Add(new KeyValuePair(name, value)); + var name = line.Substring(0, index); + if (string.IsNullOrEmpty(name)) + { + continue; } - return new OpenIddictResponse( - from parameter in parameters - group parameter by parameter.Key into grouping - let values = grouping.Select(parameter => parameter.Value) - select new KeyValuePair(grouping.Key, values.ToArray())); + var value = line.Substring(index + 1); + + parameters.Add(new KeyValuePair(name, value)); } + + return new OpenIddictResponse( + from parameter in parameters + group parameter by parameter.Key into grouping + let values = grouping.Select(parameter => parameter.Value) + select new KeyValuePair(grouping.Key, values.ToArray())); } return new OpenIddictResponse();