From 15240d8d8e36dc43381db13d4723a686b80b6e2c Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Tue, 24 Jan 2023 10:22:16 -0800 Subject: [PATCH 1/2] Remove support for case sensitive enum parsing (unused) --- Src/Fido2.Models/Converters/EnumNameMapper.cs | 33 ++---------- Src/Fido2/Extensions/EnumExtensions.cs | 17 +++--- Test/EnumExtensionTest.cs | 52 ++++++++----------- 3 files changed, 34 insertions(+), 68 deletions(-) diff --git a/Src/Fido2.Models/Converters/EnumNameMapper.cs b/Src/Fido2.Models/Converters/EnumNameMapper.cs index 56646e6d1..9e155bee8 100644 --- a/Src/Fido2.Models/Converters/EnumNameMapper.cs +++ b/Src/Fido2.Models/Converters/EnumNameMapper.cs @@ -9,8 +9,8 @@ namespace Fido2NetLib; public static class EnumNameMapper<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)] TEnum> where TEnum: struct, Enum { - private static readonly Dictionary valueToNames = GetIdToNameMap(); - private static readonly Dictionary namesToValues = Invert(valueToNames); + private static readonly Dictionary s_valueToNames = GetIdToNameMap(); + private static readonly Dictionary s_namesToValues = Invert(s_valueToNames); private static Dictionary Invert(Dictionary map) { @@ -24,42 +24,19 @@ private static Dictionary Invert(Dictionary map) return result; } - public static bool TryGetValue(string name, bool ignoreCase, out TEnum value) - { - if (namesToValues.TryGetValue(name, out value)) - { - if (!ignoreCase && !valueToNames[value].Equals(name, StringComparison.Ordinal)) - { - value = default; - - return false; - } - else - { - return true; - } - } - else - { - value = default; - - return false; - } - } - public static bool TryGetValue(string name, out TEnum value) { - return namesToValues.TryGetValue(name, out value); + return s_namesToValues.TryGetValue(name, out value); } public static string GetName(TEnum value) { - return valueToNames[value]; + return s_valueToNames[value]; } public static IEnumerable GetNames() { - return namesToValues.Keys; + return s_namesToValues.Keys; } private static Dictionary GetIdToNameMap() diff --git a/Src/Fido2/Extensions/EnumExtensions.cs b/Src/Fido2/Extensions/EnumExtensions.cs index 0b144ab3c..0d4caba05 100644 --- a/Src/Fido2/Extensions/EnumExtensions.cs +++ b/Src/Fido2/Extensions/EnumExtensions.cs @@ -10,29 +10,28 @@ public static class EnumExtensions /// /// The type of enum. /// The EnumMemberAttribute's value. - /// ignores the case when comparing values. /// TEnum. /// No XmlEnumAttribute code exists for type " + typeof(TEnum).ToString() + " corresponding to value of " + value - public static TEnum ToEnum<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)] TEnum>(this string value, bool ignoreCase = true) where TEnum : struct, Enum + public static TEnum ToEnum<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)] TEnum>(this string value) where TEnum : struct, Enum { - // Try to parse it normally on the first try - if (Enum.TryParse(value, ignoreCase, out var result)) - return result; - // Try with value from EnumMemberAttribute - if (EnumNameMapper.TryGetValue(value, ignoreCase, out result)) + if (EnumNameMapper.TryGetValue(value, out var result)) { return result; } - throw new ArgumentException($"Value '{value}' is not a valid enum name of '{typeof(TEnum)}' ({nameof(ignoreCase)}={ignoreCase}). Valid values are: {string.Join(", ", EnumNameMapper.GetNames())}."); + // Then check the enum + if (Enum.TryParse(value, out result)) + return result; + + throw new ArgumentException($"Value '{value}' is not a valid enum name of '{typeof(TEnum)}'. Valid values are: {string.Join(", ", EnumNameMapper.GetNames())}."); } /// /// Gets the EnumMemberAttribute's value from the enum's value. /// /// The type of enum. - /// The enum's value. + /// The enum's value /// string. public static string ToEnumMemberValue<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicFields)] TEnum>(this TEnum value) where TEnum : struct, Enum { diff --git a/Test/EnumExtensionTest.cs b/Test/EnumExtensionTest.cs index 07eeac8fd..756dc37ac 100644 --- a/Test/EnumExtensionTest.cs +++ b/Test/EnumExtensionTest.cs @@ -1,39 +1,29 @@ -using Fido2NetLib; -using Fido2NetLib.Objects; +using Fido2NetLib.Objects; -namespace fido2_net_lib.Test; +namespace Fido2NetLib.Test; public class EnumExtensionTest { [Fact] public void TestToEnum() { - var enumNames = Enum.GetNames(typeof(AttestationConveyancePreference)); - foreach (var enumName in enumNames) + foreach (var enumName in Enum.GetNames(typeof(AttestationConveyancePreference))) { enumName.ToEnum(); } } [Theory] - // ignoreCase true, valid - [InlineData("INDIRECT", true, false)] - [InlineData("indIrEcT", true, false)] - [InlineData("indirect", true, false)] - + // valid + [InlineData("INDIRECT", false)] + [InlineData("indIrEcT", false)] + [InlineData("indirect", false)] + [InlineData(nameof(AttestationConveyancePreference.Indirect), false)] // invalid - [InlineData("Indirect_Invalid", true, true)] - - // ignoreCase false, valid - [InlineData(nameof(AttestationConveyancePreference.Indirect), false, false)] - - // invalid - [InlineData("Indirect_Invalid", false, true)] - [InlineData("INDIRECT", false, true)] - [InlineData("indIrEcT", false, true)] - public void TestToEnumWithIgnoringCase(string value, bool ignoreCase, bool shouldThrow) + [InlineData("Indirect_Invalid", true)] + public void TestToEnumWithIgnoringCase(string value, bool shouldThrow) { - var exception = Record.Exception(() => value.ToEnum(ignoreCase)); + var exception = Record.Exception(() => value.ToEnum()); if (shouldThrow) { @@ -46,17 +36,17 @@ public void TestToEnumWithIgnoringCase(string value, bool ignoreCase, bool shoul } [Theory] - [InlineData("CROSS-PLATFORM", true, false)] // valid - [InlineData("cRoss-PlatfoRm", true, false)] // valid - [InlineData("cross-platform", true, false)] // valid - [InlineData("cross_platform", true, true)] // invalid - [InlineData("cross-platforms", true, true)] // invalid - [InlineData("CROSS_PLATFORM", true, true)] // invalid - [InlineData("CROSS-PLATFORM", false, true)] // invalid - [InlineData("cRoss-PlatfoRm", false, true)] // invalid - public void TestToEnumWithDashes(string value, bool ignoreCase, bool shouldThrow) + // valid + [InlineData("CROSS-PLATFORM", false)] + [InlineData("cRoss-PlatfoRm", false)] + [InlineData("cross-platform", false)] + // invalid + [InlineData("cross_platform", true)] + [InlineData("cross-platforms", true)] + [InlineData("CROSS_PLATFORM", true)] + public void TestToEnumWithDashes(string value, bool shouldThrow) { - var exception = Record.Exception(() => value.ToEnum(ignoreCase)); + var exception = Record.Exception(() => value.ToEnum()); if (shouldThrow) { From 0b939554b173be27a2a22713341480cdbf43d727 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Tue, 24 Jan 2023 10:41:55 -0800 Subject: [PATCH 2/2] Reference published webauthn-2 specification --- Src/Fido2.Models/CredentialCreateOptions.cs | 28 +++++++++++++------ .../AttestationConveyancePreference.cs | 4 +-- .../Objects/AuthenticatorAttachment.cs | 2 +- .../Objects/AuthenticatorTransport.cs | 13 ++++++--- .../Objects/PublicKeyCredentialType.cs | 2 +- .../Objects/ResidentKeyRequirement.cs | 2 +- .../Objects/UserVerificationRequirement.cs | 14 ++++++---- 7 files changed, 43 insertions(+), 22 deletions(-) diff --git a/Src/Fido2.Models/CredentialCreateOptions.cs b/Src/Fido2.Models/CredentialCreateOptions.cs index cfadde1c4..a47c675c9 100644 --- a/Src/Fido2.Models/CredentialCreateOptions.cs +++ b/Src/Fido2.Models/CredentialCreateOptions.cs @@ -112,6 +112,8 @@ public static CredentialCreateOptions FromJson(string json) } } +#nullable enable + public sealed class PubKeyCredParam { /// @@ -148,7 +150,6 @@ public PubKeyCredParam(COSE.Algorithm alg, PublicKeyCredentialType type = Public public static readonly PubKeyCredParam Ed25519 = new(COSE.Algorithm.EdDSA); } -#nullable enable /// /// PublicKeyCredentialRpEntity /// @@ -181,20 +182,25 @@ public PublicKeyCredentialRpEntity(string id, string name, string? icon = null) /// /// WebAuthn Relying Parties may use the AuthenticatorSelectionCriteria dictionary to specify their requirements regarding authenticator attributes. -/// https://w3c.github.io/webauthn/#dictionary-authenticatorSelection +/// https://www.w3.org/TR/webauthn-2/#dictionary-authenticatorSelection /// public class AuthenticatorSelection { /// - /// If this member is present, eligible authenticators are filtered to only authenticators attached with the specified §5.4.5 Authenticator Attachment enumeration (enum AuthenticatorAttachment). + /// If this member is present, eligible authenticators are filtered to only authenticators attached with the specified § 5.4.5 Authenticator Attachment Enumeration (enum AuthenticatorAttachment). /// [JsonPropertyName("authenticatorAttachment")] [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] public AuthenticatorAttachment? AuthenticatorAttachment { get; set; } private ResidentKeyRequirement _residentKey; + /// - /// Specifies the extent to which the Relying Party desires to create a client-side discoverable credential. For historical reasons the naming retains the deprecated “resident” terminology. The value SHOULD be a member of ResidentKeyRequirement but client platforms MUST ignore unknown values, treating an unknown value as if the member does not exist. If no value is given then the effective value is required if requireResidentKey is true or discouraged if it is false or absent. + /// Specifies the extent to which the Relying Party desires to create a client-side discoverable credential. + /// For historical reasons the naming retains the deprecated “resident” terminology. + /// The value SHOULD be a member of ResidentKeyRequirement but client platforms MUST ignore unknown values, + /// treating an unknown value as if the member does not exist. + /// If no value is given then the effective value is required if requireResidentKey is true or discouraged if it is false or absent. /// [JsonPropertyName("residentKey")] public ResidentKeyRequirement ResidentKey @@ -207,12 +213,13 @@ public ResidentKeyRequirement ResidentKey { ResidentKeyRequirement.Required => true, ResidentKeyRequirement.Preferred or ResidentKeyRequirement.Discouraged => false, - _ => throw new NotImplementedException(), + _ => throw new NotImplementedException() }; } } private bool _requireResidentKey; + /// /// This member describes the Relying Parties' requirements regarding resident credentials. If the parameter is set to true, the authenticator MUST create a client-side-resident public key credential source when creating a public key credential. /// @@ -245,20 +252,25 @@ public bool RequireResidentKey public class Fido2User { /// - /// Required. A human-friendly identifier for a user account. It is intended only for display, i.e., aiding the user in determining the difference between user accounts with similar displayNames. For example, "alexm", "alex.p.mueller@example.com" or "+14255551234". https://w3c.github.io/webauthn/#dictdef-publickeycredentialentity + /// Required. A human-friendly identifier for a user account. + /// It is intended only for display, i.e., aiding the user in determining the difference between user accounts with similar displayNames. + /// For example, "alexm", "alex.p.mueller@example.com" or "+14255551234". https://w3c.github.io/webauthn/#dictdef-publickeycredentialentity /// [JsonPropertyName("name")] public string Name { get; set; } /// - /// The user handle of the user account entity. To ensure secure operation, authentication and authorization decisions MUST be made on the basis of this id member, not the displayName nor name members + /// The user handle of the user account entity. + /// To ensure secure operation, authentication and authorization decisions MUST be made on the basis of this id member, not the displayName nor name members /// [JsonPropertyName("id")] [JsonConverter(typeof(Base64UrlConverter))] public byte[] Id { get; set; } /// - /// A human-friendly name for the user account, intended only for display. For example, "Alex P. Müller" or "田中 倫". The Relying Party SHOULD let the user choose this, and SHOULD NOT restrict the choice more than necessary. + /// A human-friendly name for the user account, intended only for display. + /// For example, "Alex P. Müller" or "田中 倫". + /// The Relying Party SHOULD let the user choose this, and SHOULD NOT restrict the choice more than necessary. /// [JsonPropertyName("displayName")] public string DisplayName { get; set; } diff --git a/Src/Fido2.Models/Objects/AttestationConveyancePreference.cs b/Src/Fido2.Models/Objects/AttestationConveyancePreference.cs index b241bc191..2fccebe00 100644 --- a/Src/Fido2.Models/Objects/AttestationConveyancePreference.cs +++ b/Src/Fido2.Models/Objects/AttestationConveyancePreference.cs @@ -4,8 +4,8 @@ namespace Fido2NetLib.Objects; /// -/// AttestationConveyancePreference. -/// https://w3c.github.io/webauthn/#attestation-convey +/// AttestationConveyancePreference +/// https://www.w3.org/TR/webauthn-2/#enum-attestation-convey /// [JsonConverter(typeof(FidoEnumConverter))] public enum AttestationConveyancePreference diff --git a/Src/Fido2.Models/Objects/AuthenticatorAttachment.cs b/Src/Fido2.Models/Objects/AuthenticatorAttachment.cs index fe3ffdb4d..9833a353d 100644 --- a/Src/Fido2.Models/Objects/AuthenticatorAttachment.cs +++ b/Src/Fido2.Models/Objects/AuthenticatorAttachment.cs @@ -10,7 +10,7 @@ namespace Fido2NetLib.Objects; /// /// /// Note: An authenticator attachment modality selection option is available only in the [[Create]](origin, options, sameOriginWithAncestors) operation. The Relying Party may use it to, for example, ensure the user has a roaming credential for authenticating on another client device; or to specifically register a platform credential for easier reauthentication using a particular client device. The [[DiscoverFromExternalSource]](origin, options, sameOriginWithAncestors) operation has no authenticator attachment modality selection option, so the Relying Party SHOULD accept any of the user’s registered credentials. The client and user will then use whichever is available and convenient at the time. -/// https://w3c.github.io/webauthn/#attachment +/// https://www.w3.org/TR/webauthn-2/#enum-attachment /// [JsonConverter(typeof(FidoEnumConverter))] public enum AuthenticatorAttachment diff --git a/Src/Fido2.Models/Objects/AuthenticatorTransport.cs b/Src/Fido2.Models/Objects/AuthenticatorTransport.cs index 1d7a75dc8..53b1bda45 100644 --- a/Src/Fido2.Models/Objects/AuthenticatorTransport.cs +++ b/Src/Fido2.Models/Objects/AuthenticatorTransport.cs @@ -4,8 +4,12 @@ namespace Fido2NetLib.Objects; /// -/// Authenticators may implement various transports for communicating with clients. This enumeration defines hints as to how clients might communicate with a particular authenticator in order to obtain an assertion for a specific credential. Note that these hints represent the WebAuthn Relying Party's best belief as to how an authenticator may be reached. A Relying Party may obtain a list of transports hints from some attestation statement formats or via some out-of-band mechanism; it is outside the scope of this specification to define that mechanism. -/// https://w3c.github.io/webauthn/#transport +/// Authenticators may implement various transports for communicating with clients. +/// This enumeration defines hints as to how clients might communicate with a particular +/// authenticator in order to obtain an assertion for a specific credential. +/// Note that these hints represent the WebAuthn Relying Party's best belief as to how an authenticator may be reached. +/// A Relying Party will typically learn of the supported transports for a public key credential via getTransports(). +/// https://www.w3.org/TR/webauthn-2/#enum-transport /// [JsonConverter(typeof(FidoEnumConverter))] public enum AuthenticatorTransport @@ -23,13 +27,14 @@ public enum AuthenticatorTransport Nfc, /// - /// Indicates the respective authenticator can be contacted over Bluetooth Smart(Bluetooth Low Energy / BLE) + /// Indicates the respective authenticator can be contacted over Bluetooth Smart (Bluetooth Low Energy / BLE). /// [EnumMember(Value = "ble")] Ble, /// - /// Indicates the respective authenticator is contacted using a client device-specific transport.These authenticators are not removable from the client device. + /// Indicates the respective authenticator is contacted using a client device-specific transport, i.e., it is a platform authenticator. + /// These authenticators are not removable from the client device. /// [EnumMember(Value = "internal")] Internal, diff --git a/Src/Fido2.Models/Objects/PublicKeyCredentialType.cs b/Src/Fido2.Models/Objects/PublicKeyCredentialType.cs index bd298d3a6..74930bb2d 100644 --- a/Src/Fido2.Models/Objects/PublicKeyCredentialType.cs +++ b/Src/Fido2.Models/Objects/PublicKeyCredentialType.cs @@ -5,7 +5,7 @@ namespace Fido2NetLib.Objects; /// /// PublicKeyCredentialType. -/// https://w3c.github.io/webauthn/#enumdef-publickeycredentialtype +/// https://www.w3.org/TR/webauthn-2/#enum-credentialType /// [JsonConverter(typeof(FidoEnumConverter))] public enum PublicKeyCredentialType diff --git a/Src/Fido2.Models/Objects/ResidentKeyRequirement.cs b/Src/Fido2.Models/Objects/ResidentKeyRequirement.cs index 263b3cea1..21763d25e 100644 --- a/Src/Fido2.Models/Objects/ResidentKeyRequirement.cs +++ b/Src/Fido2.Models/Objects/ResidentKeyRequirement.cs @@ -5,7 +5,7 @@ namespace Fido2NetLib.Objects; /// /// This enumeration’s values describe the Relying Party's requirements for client-side discoverable credentials (formerly known as resident credentials or resident keys). -/// https://w3c.github.io/webauthn/#enum-residentKeyRequirement +/// https://www.w3.org/TR/webauthn-2/#enum-residentKeyRequirement /// [JsonConverter(typeof(FidoEnumConverter))] public enum ResidentKeyRequirement diff --git a/Src/Fido2.Models/Objects/UserVerificationRequirement.cs b/Src/Fido2.Models/Objects/UserVerificationRequirement.cs index dcec64276..e8732e67d 100644 --- a/Src/Fido2.Models/Objects/UserVerificationRequirement.cs +++ b/Src/Fido2.Models/Objects/UserVerificationRequirement.cs @@ -4,26 +4,30 @@ namespace Fido2NetLib.Objects; /// -/// A WebAuthn Relying Party may require user verification for some of its operations but not for others, and may use this type to express its needs. -/// https://w3c.github.io/webauthn/#enumdef-userverificationrequirement +/// A WebAuthn Relying Party may require user verification for some of its operations but not for others, +/// and may use this type to express its needs. +/// https://www.w3.org/TR/webauthn-2/#enumdef-userverificationrequirement /// [JsonConverter(typeof(FidoEnumConverter))] public enum UserVerificationRequirement { /// - /// This value indicates that the Relying Party requires user verification for the operation and will fail the operation if the response does not have the UV flag set. + /// This value indicates that the Relying Party requires user verification for the operation + /// and will fail the operation if the response does not have the UV flag set. /// [EnumMember(Value = "required")] Required, /// - /// This value indicates that the Relying Party prefers user verification for the operation if possible, but will not fail the operation if the response does not have the UV flag set. + /// This value indicates that the Relying Party prefers user verification for the operation if possible, + /// but will not fail the operation if the response does not have the UV flag set. /// [EnumMember(Value = "preferred")] Preferred, /// - /// This value indicates that the Relying Party does not want user verification employed during the operation(e.g., in the interest of minimizing disruption to the user interaction flow). + /// This value indicates that the Relying Party does not want user verification employed during the operation + /// (e.g., in the interest of minimizing disruption to the user interaction flow). /// [EnumMember(Value = "discouraged")] Discouraged