From 177932327f7d71b820b9640dbf5bf5dd3118c186 Mon Sep 17 00:00:00 2001 From: Oleksii Holub <1935960+Tyrrrz@users.noreply.github.com> Date: Fri, 29 Sep 2023 14:24:54 +0300 Subject: [PATCH 1/6] Rework `PasswordlessClient` --- src/Passwordless/PasswordlessClient.cs | 59 +++++++++---------- .../PasswordlessDelegatingHandler.cs | 31 ---------- src/Passwordless/PasswordlessExtensions.cs | 9 --- src/Passwordless/PasswordlessHttpHandler.cs | 48 +++++++++++++++ .../ServiceCollectionExtensions.cs | 33 ++--------- 5 files changed, 79 insertions(+), 101 deletions(-) delete mode 100644 src/Passwordless/PasswordlessDelegatingHandler.cs delete mode 100644 src/Passwordless/PasswordlessExtensions.cs create mode 100644 src/Passwordless/PasswordlessHttpHandler.cs diff --git a/src/Passwordless/PasswordlessClient.cs b/src/Passwordless/PasswordlessClient.cs index 6cb7c20..96214bb 100644 --- a/src/Passwordless/PasswordlessClient.cs +++ b/src/Passwordless/PasswordlessClient.cs @@ -11,46 +11,43 @@ namespace Passwordless; [DebuggerDisplay("{DebuggerToString(),nq}")] public class PasswordlessClient : IPasswordlessClient, IDisposable { - private readonly HttpClient _client; - private readonly bool _disposeClient; + private readonly HttpClient _http; /// /// Initializes an instance of . /// - public static PasswordlessClient Create(PasswordlessOptions options, IHttpClientFactory factory) + private PasswordlessClient(HttpClient http, bool disposeClient, PasswordlessOptions options) { - var client = factory.CreateClient(); - client.BaseAddress = new Uri(options.ApiUrl); - client.DefaultRequestHeaders.Add("ApiSecret", options.ApiSecret); - return new PasswordlessClient(client); + _http = new HttpClient(new PasswordlessHttpHandler(http, disposeClient)) + { + BaseAddress = new Uri(options.ApiUrl), + DefaultRequestHeaders = + { + {"ApiSecret", options.ApiSecret} + } + }; } /// /// Initializes an instance of . /// - public PasswordlessClient(PasswordlessOptions passwordlessOptions) + public PasswordlessClient(HttpClient http, PasswordlessOptions options) + : this(http, false, options) { - _client = new HttpClient - { - BaseAddress = new Uri(passwordlessOptions.ApiUrl), - }; - _client.DefaultRequestHeaders.Add("ApiSecret", passwordlessOptions.ApiSecret); - _disposeClient = true; } /// /// Initializes an instance of . /// - public PasswordlessClient(HttpClient client) + public PasswordlessClient(PasswordlessOptions options) + : this(new HttpClient(), true, options) { - _client = client; - _disposeClient = false; } /// public async Task SetAliasAsync(SetAliasRequest request, CancellationToken cancellationToken) { - using var response = await _client.PostAsJsonAsync("alias", + using var response = await _http.PostAsJsonAsync("alias", request, PasswordlessSerializerContext.Default.SetAliasRequest, cancellationToken); @@ -61,7 +58,7 @@ public async Task SetAliasAsync(SetAliasRequest request, CancellationToken cance /// public async Task CreateRegisterTokenAsync(RegisterOptions registerOptions, CancellationToken cancellationToken = default) { - using var response = await _client.PostAsJsonAsync("register/token", + using var response = await _http.PostAsJsonAsync("register/token", registerOptions, PasswordlessSerializerContext.Default.RegisterOptions, cancellationToken); @@ -83,7 +80,7 @@ public async Task CreateRegisterTokenAsync(RegisterOption // We just want to return null if there is a problem. request.SkipErrorHandling(); - using var response = await _client.SendAsync(request, cancellationToken); + using var response = await _http.SendAsync(request, cancellationToken); if (response.IsSuccessStatusCode) { @@ -100,7 +97,7 @@ public async Task CreateRegisterTokenAsync(RegisterOption /// public async Task DeleteUserAsync(string userId, CancellationToken cancellationToken = default) { - using var response = await _client.PostAsJsonAsync("users/delete", + using var response = await _http.PostAsJsonAsync("users/delete", new DeleteUserRequest(userId), PasswordlessSerializerContext.Default.DeleteUserRequest, cancellationToken); @@ -109,7 +106,7 @@ public async Task DeleteUserAsync(string userId, CancellationToken cancellationT /// public async Task> ListUsersAsync(CancellationToken cancellationToken = default) { - var response = await _client.GetFromJsonAsync( + var response = await _http.GetFromJsonAsync( "users/list", PasswordlessSerializerContext.Default.ListResponsePasswordlessUserSummary, cancellationToken); @@ -120,7 +117,7 @@ public async Task> ListUsersAsync(Cancell /// public async Task> ListAliasesAsync(string userId, CancellationToken cancellationToken = default) { - var response = await _client.GetFromJsonAsync( + var response = await _http.GetFromJsonAsync( $"alias/list?userid={userId}", PasswordlessSerializerContext.Default.ListResponseAliasPointer, cancellationToken); @@ -131,7 +128,7 @@ public async Task> ListAliasesAsync(string userId, C /// public async Task> ListCredentialsAsync(string userId, CancellationToken cancellationToken = default) { - var response = await _client.GetFromJsonAsync( + var response = await _http.GetFromJsonAsync( $"credentials/list?userid={userId}", PasswordlessSerializerContext.Default.ListResponseCredential, cancellationToken); @@ -142,7 +139,7 @@ public async Task> ListCredentialsAsync(string userId, /// public async Task DeleteCredentialAsync(string id, CancellationToken cancellationToken = default) { - using var response = await _client.PostAsJsonAsync("credentials/delete", + using var response = await _http.PostAsJsonAsync("credentials/delete", new DeleteCredentialRequest(id), PasswordlessSerializerContext.Default.DeleteCredentialRequest, cancellationToken); @@ -157,7 +154,7 @@ public async Task DeleteCredentialAsync(byte[] id, CancellationToken cancellatio /// public async Task GetUsersCountAsync(CancellationToken cancellationToken = default) { - return (await _client.GetFromJsonAsync( + return (await _http.GetFromJsonAsync( "users/count", PasswordlessSerializerContext.Default.UsersCount, cancellationToken))!; @@ -167,8 +164,8 @@ private string DebuggerToString() { var sb = new StringBuilder(); sb.Append("ApiUrl = "); - sb.Append(_client.BaseAddress); - if (_client.DefaultRequestHeaders.TryGetValues("ApiSecret", out var values)) + sb.Append(_http.BaseAddress); + if (_http.DefaultRequestHeaders.TryGetValues("ApiSecret", out var values)) { var apiSecret = values.First(); if (apiSecret.Length > 5) @@ -200,9 +197,7 @@ public void Dispose() /// protected virtual void Dispose(bool disposing) { - if (disposing && _disposeClient) - { - _client.Dispose(); - } + if (disposing) + _http.Dispose(); } } \ No newline at end of file diff --git a/src/Passwordless/PasswordlessDelegatingHandler.cs b/src/Passwordless/PasswordlessDelegatingHandler.cs deleted file mode 100644 index bdc0387..0000000 --- a/src/Passwordless/PasswordlessDelegatingHandler.cs +++ /dev/null @@ -1,31 +0,0 @@ -using System.Net.Http.Json; -using Passwordless.Helpers; - -namespace Passwordless; - -internal class PasswordlessDelegatingHandler : DelegatingHandler -{ - protected override async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) - { - var response = await base.SendAsync(request, cancellationToken); - - if (request.ShouldSkipErrorHandling()) - { - return response; - } - - if (!response.IsSuccessStatusCode - && string.Equals(response.Content.Headers.ContentType?.MediaType, "application/problem+json", StringComparison.OrdinalIgnoreCase)) - { - // Attempt to read problem details - var problemDetails = await response.Content.ReadFromJsonAsync( - PasswordlessSerializerContext.Default.PasswordlessProblemDetails, - cancellationToken); - - // Throw exception - throw new PasswordlessApiException(problemDetails!); - } - - return response; - } -} \ No newline at end of file diff --git a/src/Passwordless/PasswordlessExtensions.cs b/src/Passwordless/PasswordlessExtensions.cs deleted file mode 100644 index cfa479f..0000000 --- a/src/Passwordless/PasswordlessExtensions.cs +++ /dev/null @@ -1,9 +0,0 @@ -namespace Passwordless; - -public static class PasswordlessExtensions -{ - public static string ToBase64Url(this byte[] bytes) - { - return Base64Url.Encode(bytes); - } -} \ No newline at end of file diff --git a/src/Passwordless/PasswordlessHttpHandler.cs b/src/Passwordless/PasswordlessHttpHandler.cs new file mode 100644 index 0000000..fe05c1b --- /dev/null +++ b/src/Passwordless/PasswordlessHttpHandler.cs @@ -0,0 +1,48 @@ +using System.Net.Http.Json; +using Passwordless.Helpers; + +namespace Passwordless; + +internal class PasswordlessHttpHandler : HttpMessageHandler +{ + // Externally provided HTTP Client + private readonly HttpClient _http; + private readonly bool _disposeClient; + + public PasswordlessHttpHandler(HttpClient http, bool disposeClient = false) + { + _http = http; + _disposeClient = disposeClient; + } + + protected override async Task SendAsync( + HttpRequestMessage request, + CancellationToken cancellationToken) + { + var response = await _http.SendAsync(request, HttpCompletionOption.ResponseHeadersRead, cancellationToken); + + // On failed requests, check if responded with ProblemDetails and provide a nicer error if so + if (!request.ShouldSkipErrorHandling() && + !response.IsSuccessStatusCode && + string.Equals(response.Content.Headers.ContentType?.MediaType, + "application/problem+json", + StringComparison.OrdinalIgnoreCase)) + { + var problemDetails = await response.Content.ReadFromJsonAsync( + PasswordlessSerializerContext.Default.PasswordlessProblemDetails, + cancellationToken + ); + + if (problemDetails is not null) + throw new PasswordlessApiException(problemDetails); + } + + return response; + } + + protected override void Dispose(bool disposing) + { + if (disposing && _disposeClient) + _http.Dispose(); + } +} \ No newline at end of file diff --git a/src/Passwordless/ServiceCollectionExtensions.cs b/src/Passwordless/ServiceCollectionExtensions.cs index b228445..a2debbb 100644 --- a/src/Passwordless/ServiceCollectionExtensions.cs +++ b/src/Passwordless/ServiceCollectionExtensions.cs @@ -13,45 +13,20 @@ public static class ServiceCollectionExtensions /// /// Adds and configures Passwordless-related services. /// - public static IServiceCollection AddPasswordlessSdk(this IServiceCollection services, Action configureOptions) + public static IServiceCollection AddPasswordlessSdk( + this IServiceCollection services, + Action configureOptions) { services.AddOptions() .Configure(configureOptions) .PostConfigure(options => options.ApiUrl ??= PasswordlessOptions.CloudApiUrl) .Validate(options => !string.IsNullOrEmpty(options.ApiSecret), "Passwordless: Missing ApiSecret"); - services.AddPasswordlessClientCore((sp, client) => - { - var options = sp.GetRequiredService>().Value; - - client.BaseAddress = new Uri(options.ApiUrl); - client.DefaultRequestHeaders.Add("ApiSecret", options.ApiSecret); - }); + services.AddHttpClient(); // TODO: Get rid of this service, all consumers should use the interface services.AddTransient(sp => (PasswordlessClient)sp.GetRequiredService()); return services; } - - /// - /// Helper method for making custom typed HttpClient implementations that also have - /// the inner handler for throwing fancy exceptions. Not intended for public use, - /// hence the hiding of it in IDE's. - /// - /// - /// This method signature is subject to change without major version bump/announcement. - /// - internal static IServiceCollection AddPasswordlessClientCore<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] TClient, [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] TImplementation>(this IServiceCollection services, Action configureClient) - where TClient : class - where TImplementation : class, TClient - { - services.AddTransient(); - - services - .AddHttpClient(configureClient) - .AddHttpMessageHandler(); - - return services; - } } \ No newline at end of file From ebb662de223c4b05b467612f5242a28d68127165 Mon Sep 17 00:00:00 2001 From: Oleksii Holub <1935960+Tyrrrz@users.noreply.github.com> Date: Fri, 29 Sep 2023 14:44:10 +0300 Subject: [PATCH 2/6] Remove useless comment --- src/Passwordless/PasswordlessClient.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Passwordless/PasswordlessClient.cs b/src/Passwordless/PasswordlessClient.cs index 96214bb..269fc99 100644 --- a/src/Passwordless/PasswordlessClient.cs +++ b/src/Passwordless/PasswordlessClient.cs @@ -13,9 +13,6 @@ public class PasswordlessClient : IPasswordlessClient, IDisposable { private readonly HttpClient _http; - /// - /// Initializes an instance of . - /// private PasswordlessClient(HttpClient http, bool disposeClient, PasswordlessOptions options) { _http = new HttpClient(new PasswordlessHttpHandler(http, disposeClient)) From ea0db54c2995306442c9fb2f9e71770ba8619d2e Mon Sep 17 00:00:00 2001 From: Oleksii Holub <1935960+Tyrrrz@users.noreply.github.com> Date: Fri, 29 Sep 2023 14:45:21 +0300 Subject: [PATCH 3/6] Remove some unused usings --- src/Passwordless/ServiceCollectionExtensions.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Passwordless/ServiceCollectionExtensions.cs b/src/Passwordless/ServiceCollectionExtensions.cs index a2debbb..f2b3678 100644 --- a/src/Passwordless/ServiceCollectionExtensions.cs +++ b/src/Passwordless/ServiceCollectionExtensions.cs @@ -1,5 +1,3 @@ -using System.Diagnostics.CodeAnalysis; -using Microsoft.Extensions.Options; using Passwordless; // This is a trick to always show up in a class when people are registering services From 37bdd302a088f54c6de0a6371050e3e3d8afcf96 Mon Sep 17 00:00:00 2001 From: Oleksii Holub <1935960+Tyrrrz@users.noreply.github.com> Date: Fri, 29 Sep 2023 14:48:24 +0300 Subject: [PATCH 4/6] Clean up `DebuggerToString()` --- src/Passwordless/PasswordlessClient.cs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/Passwordless/PasswordlessClient.cs b/src/Passwordless/PasswordlessClient.cs index 269fc99..b2bf9d6 100644 --- a/src/Passwordless/PasswordlessClient.cs +++ b/src/Passwordless/PasswordlessClient.cs @@ -12,6 +12,7 @@ namespace Passwordless; public class PasswordlessClient : IPasswordlessClient, IDisposable { private readonly HttpClient _http; + private readonly PasswordlessOptions _options; private PasswordlessClient(HttpClient http, bool disposeClient, PasswordlessOptions options) { @@ -23,6 +24,8 @@ private PasswordlessClient(HttpClient http, bool disposeClient, PasswordlessOpti {"ApiSecret", options.ApiSecret} } }; + + _options = options; } /// @@ -160,17 +163,18 @@ public async Task GetUsersCountAsync(CancellationToken cancellationT private string DebuggerToString() { var sb = new StringBuilder(); + sb.Append("ApiUrl = "); - sb.Append(_http.BaseAddress); - if (_http.DefaultRequestHeaders.TryGetValues("ApiSecret", out var values)) + sb.Append(_options.ApiUrl); + + if (!string.IsNullOrEmpty(_options.ApiSecret)) { - var apiSecret = values.First(); - if (apiSecret.Length > 5) + if (_options.ApiSecret.Length > 5) { sb.Append(' '); sb.Append("ApiSecret = "); sb.Append("***"); - sb.Append(apiSecret.Substring(apiSecret.Length - 4)); + sb.Append(_options.ApiSecret.Substring(_options.ApiSecret.Length - 4)); } } else From a9fff15df92711ae0f54563758458c080309bac6 Mon Sep 17 00:00:00 2001 From: Oleksii Holub <1935960+Tyrrrz@users.noreply.github.com> Date: Fri, 29 Sep 2023 14:50:11 +0300 Subject: [PATCH 5/6] Always dispose handler --- src/Passwordless/PasswordlessClient.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Passwordless/PasswordlessClient.cs b/src/Passwordless/PasswordlessClient.cs index b2bf9d6..941bf05 100644 --- a/src/Passwordless/PasswordlessClient.cs +++ b/src/Passwordless/PasswordlessClient.cs @@ -16,7 +16,7 @@ public class PasswordlessClient : IPasswordlessClient, IDisposable private PasswordlessClient(HttpClient http, bool disposeClient, PasswordlessOptions options) { - _http = new HttpClient(new PasswordlessHttpHandler(http, disposeClient)) + _http = new HttpClient(new PasswordlessHttpHandler(http, disposeClient), true) { BaseAddress = new Uri(options.ApiUrl), DefaultRequestHeaders = From ccf36a6b1cc8b09854eea195d1af863e8904a613 Mon Sep 17 00:00:00 2001 From: Oleksii Holub <1935960+Tyrrrz@users.noreply.github.com> Date: Fri, 29 Sep 2023 14:52:42 +0300 Subject: [PATCH 6/6] Remove unused polyfills --- src/Passwordless/Polyfill.CodeAnalysis.cs | 144 ---------------------- 1 file changed, 144 deletions(-) delete mode 100644 src/Passwordless/Polyfill.CodeAnalysis.cs diff --git a/src/Passwordless/Polyfill.CodeAnalysis.cs b/src/Passwordless/Polyfill.CodeAnalysis.cs deleted file mode 100644 index 559ed10..0000000 --- a/src/Passwordless/Polyfill.CodeAnalysis.cs +++ /dev/null @@ -1,144 +0,0 @@ -#if !NET6_0_OR_GREATER - -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -namespace System.Diagnostics.CodeAnalysis; - -/// -/// Indicates that certain members on a specified are accessed dynamically, -/// for example through . -/// -/// -/// This allows tools to understand which members are being accessed during the execution -/// of a program. -/// -/// This attribute is valid on members whose type is or . -/// -/// When this attribute is applied to a location of type , the assumption is -/// that the string represents a fully qualified type name. -/// -/// When this attribute is applied to a class, interface, or struct, the members specified -/// can be accessed dynamically on instances returned from calling -/// on instances of that class, interface, or struct. -/// -/// If the attribute is applied to a method it's treated as a special case and it implies -/// the attribute should be applied to the "this" parameter of the method. As such the attribute -/// should only be used on instance methods of types assignable to System.Type (or string, but no methods -/// will use it there). -/// -[AttributeUsage( - AttributeTargets.Field | AttributeTargets.ReturnValue | AttributeTargets.GenericParameter | - AttributeTargets.Parameter | AttributeTargets.Property | AttributeTargets.Method | - AttributeTargets.Class | AttributeTargets.Interface | AttributeTargets.Struct, - Inherited = false)] -internal sealed class DynamicallyAccessedMembersAttribute : Attribute -{ - /// - /// Initializes a new instance of the class - /// with the specified member types. - /// - /// The types of members dynamically accessed. - public DynamicallyAccessedMembersAttribute(DynamicallyAccessedMemberTypes memberTypes) - { - MemberTypes = memberTypes; - } - - /// - /// Gets the which specifies the type - /// of members dynamically accessed. - /// - public DynamicallyAccessedMemberTypes MemberTypes { get; } -} - -/// -/// Specifies the types of members that are dynamically accessed. -/// -/// This enumeration has a attribute that allows a -/// bitwise combination of its member values. -/// -[Flags] -internal enum DynamicallyAccessedMemberTypes -{ - /// - /// Specifies no members. - /// - None = 0, - - /// - /// Specifies the default, parameterless public constructor. - /// - PublicParameterlessConstructor = 0x0001, - - /// - /// Specifies all public constructors. - /// - PublicConstructors = 0x0002 | PublicParameterlessConstructor, - - /// - /// Specifies all non-public constructors. - /// - NonPublicConstructors = 0x0004, - - /// - /// Specifies all public methods. - /// - PublicMethods = 0x0008, - - /// - /// Specifies all non-public methods. - /// - NonPublicMethods = 0x0010, - - /// - /// Specifies all public fields. - /// - PublicFields = 0x0020, - - /// - /// Specifies all non-public fields. - /// - NonPublicFields = 0x0040, - - /// - /// Specifies all public nested types. - /// - PublicNestedTypes = 0x0080, - - /// - /// Specifies all non-public nested types. - /// - NonPublicNestedTypes = 0x0100, - - /// - /// Specifies all public properties. - /// - PublicProperties = 0x0200, - - /// - /// Specifies all non-public properties. - /// - NonPublicProperties = 0x0400, - - /// - /// Specifies all public events. - /// - PublicEvents = 0x0800, - - /// - /// Specifies all non-public events. - /// - NonPublicEvents = 0x1000, - - /// - /// Specifies all interfaces implemented by the type. - /// - Interfaces = 0x2000, - - /// - /// Specifies all members. - /// - All = ~None -} - -#endif \ No newline at end of file