[dotnet] Fix Virtual Authenticator removal, annotate NRT

Author: RenderMichael

dotnet/src/webdriver/VirtualAuth/Credential.cs

@@ -18,31 +18,30 @@
 // </copyright>
 
 using OpenQA.Selenium.Internal;
+using System;
 using System.Collections.Generic;
 
+#nullable enable
+
 namespace OpenQA.Selenium.VirtualAuth
 {
     /// <summary>
     /// A credential stored in a virtual authenticator.
-    /// Refer https://w3c.github.io/webauthn/#credential-parameters
+    /// Refer <see href="https://w3c.github.io/webauthn/#credential-parameters"/>
     /// </summary>
-    public class Credential
+    public sealed class Credential
     {
         private readonly byte[] id;
-        private readonly bool isResidentCredential;
-        private readonly string rpId;
-        private readonly string privateKey;
-        private readonly byte[] userHandle;
-        private readonly int signCount;
+        private readonly byte[]? userHandle;
 
-        private Credential(byte[] id, bool isResidentCredential, string rpId, string privateKey, byte[] userHandle, int signCount)
+        private Credential(byte[] id, bool isResidentCredential, string? rpId, string privateKey, byte[]? userHandle, int signCount)
         {
-            this.id = id;
-            this.isResidentCredential = isResidentCredential;
-            this.rpId = rpId;
-            this.privateKey = privateKey;
+            this.id = id ?? throw new ArgumentNullException(nameof(id));
+            this.IsResidentCredential = isResidentCredential;
+            this.RpId = rpId;
+            this.PrivateKey = privateKey ?? throw new ArgumentNullException(nameof(privateKey));
             this.userHandle = userHandle;
-            this.signCount = signCount;
+            this.SignCount = signCount;
         }
 
         /// <summary>
@@ -53,6 +52,7 @@ private Credential(byte[] id, bool isResidentCredential, string rpId, string pri
         /// <param name="privateKey">The private Key for the credentials.</param>
         /// <param name="signCount">The signature counter for the credentials.</param>
         /// <returns>The created instance of the Credential class.</returns>
+        /// <exception cref="ArgumentNullException">If <paramref name="id"/> or <paramref name="privateKey"/> are <see langword="null"/>.</exception>
         public static Credential CreateNonResidentCredential(byte[] id, string rpId, string privateKey, int signCount)
         {
             return new Credential(id, false, rpId, privateKey, null, signCount);
@@ -67,6 +67,7 @@ public static Credential CreateNonResidentCredential(byte[] id, string rpId, str
         /// <param name="userHandle">The user handle associated to the credential.</param>
         /// <param name="signCount">The signature counter for the credentials.</param>
         /// <returns>The created instance of the Credential class.</returns>
+        /// <exception cref="ArgumentNullException">If <paramref name="id"/> or <paramref name="privateKey"/> are <see langword="null"/>.</exception>
         public static Credential CreateResidentCredential(byte[] id, string rpId, string privateKey, byte[] userHandle, int signCount)
         {
             return new Credential(id, true, rpId, privateKey, userHandle, signCount);
@@ -75,50 +76,32 @@ public static Credential CreateResidentCredential(byte[] id, string rpId, string
         /// <summary>
         /// Gets the byte array of the ID of the credential.
         /// </summary>
-        public byte[] Id
-        {
-            get { return (byte[])id.Clone(); }
-        }
+        public byte[] Id => (byte[])id.Clone();
 
         /// <summary>
         /// Gets a value indicating whether this Credential is a resident credential.
         /// </summary>
-        public bool IsResidentCredential
-        {
-            get { return this.isResidentCredential; }
-        }
+        public bool IsResidentCredential { get; }
 
         /// <summary>
         /// Gets the ID of the relying party of this credential.
         /// </summary>
-        public string RpId
-        {
-            get { return this.rpId; }
-        }
+        public string? RpId { get; }
 
         /// <summary>
         /// Gets the private key of the credential.
         /// </summary>
-        public string PrivateKey
-        {
-            get { return this.privateKey; }
-        }
+        public string PrivateKey { get; }
 
         /// <summary>
         /// Gets the user handle of the credential.
         /// </summary>
-        public byte[] UserHandle
-        {
-            get { return userHandle == null ? null : (byte[])userHandle.Clone(); }
-        }
+        public byte[]? UserHandle => (byte[]?)userHandle?.Clone();
 
         /// <summary>
         /// Gets the signature counter associated to the public key credential source.
         /// </summary>
-        public int SignCount
-        {
-            get { return this.signCount; }
-        }
+        public int SignCount { get; }
 
         /// <summary>
         /// Creates a Credential instance from a dictionary of values.
@@ -127,13 +110,14 @@ public int SignCount
         /// <returns>The created instance of the Credential.</returns>
         public static Credential FromDictionary(Dictionary<string, object> dictionary)
         {
-            return new Credential(
-                Base64UrlEncoder.DecodeBytes((string)dictionary["credentialId"]),
-                (bool)dictionary["isResidentCredential"],
-                dictionary.ContainsKey("rpId") ? (string)dictionary["rpId"] : null,
-                (string)dictionary["privateKey"],
-                dictionary.ContainsKey("userHandle") ? Base64UrlEncoder.DecodeBytes((string)dictionary["userHandle"]) : null,
-                (int)((long)dictionary["signCount"]));
+            byte[] id = Base64UrlEncoder.DecodeBytes((string)dictionary["credentialId"]);
+            bool isResidentCredential = (bool)dictionary["isResidentCredential"];
+            string? rpId = dictionary.TryGetValue("rpId", out object? r) ? (string)r : null;
+            string privateKey = (string)dictionary["privateKey"];
+            byte[]? userHandle = dictionary.TryGetValue("userHandle", out object? u) ? Base64UrlEncoder.DecodeBytes((string)u) : null;
+            int signCount = (int)(long)dictionary["signCount"];
+
+            return new Credential(id, isResidentCredential, rpId, privateKey, userHandle, signCount);
         }
 
         /// <summary>
@@ -145,11 +129,15 @@ public Dictionary<string, object> ToDictionary()
             Dictionary<string, object> toReturn = new Dictionary<string, object>();
 
             toReturn["credentialId"] = Base64UrlEncoder.Encode(this.id);
-            toReturn["isResidentCredential"] = this.isResidentCredential;
-            toReturn["rpId"] = this.rpId;
-            toReturn["privateKey"] = this.privateKey;
-            toReturn["signCount"] = this.signCount;
-            if (this.userHandle != null)
+            toReturn["isResidentCredential"] = this.IsResidentCredential;
+            if (this.RpId is not null)
+            {
+                toReturn["rpId"] = this.RpId;
+            }
+
+            toReturn["privateKey"] = this.PrivateKey;
+            toReturn["signCount"] = this.SignCount;
+            if (this.userHandle is not null)
             {
                 toReturn["userHandle"] = Base64UrlEncoder.Encode(this.userHandle);
             }

dotnet/src/webdriver/VirtualAuth/IHasVirtualAuthenticator.cs

@@ -17,8 +17,11 @@
 // under the License.
 // </copyright>
 
+using System;
 using System.Collections.Generic;
 
+#nullable enable
+
 namespace OpenQA.Selenium.VirtualAuth
 {
     /// <summary>
@@ -31,18 +34,23 @@ public interface IHasVirtualAuthenticator
         /// </summary>
         /// <param name="options">The VirtualAuthenticatorOptions to use in creating the authenticator.</param>
         /// <returns>The ID of the added virtual authenticator.</returns>
+        /// <exception cref="ArgumentNullException">If <paramref name="options"/> is <see langword="null"/>.</exception>
         string AddVirtualAuthenticator(VirtualAuthenticatorOptions options);
 
         /// <summary>
         /// Removes a virtual authenticator.
         /// </summary>
         /// <param name="id">The ID of the virtual authenticator to remove.</param>
+        /// <exception cref="ArgumentNullException">If <paramref name="id"/> is <see langword="null"/>.</exception>
+        /// <exception cref="WebDriverArgumentException">If the specified virtual authenticator does not exist.</exception>
         void RemoveVirtualAuthenticator(string id);
 
         /// <summary>
         /// Adds a credential to the virtual authenticator.
         /// </summary>
         /// <param name="credential">The credential to add to the authenticator.</param>
+        /// <exception cref="ArgumentNullException">If <paramref name="credential"/> is <see langword="null"/>.</exception>
+        /// <exception cref="InvalidOperationException">If a Virtual Authenticator has not been added yet.</exception>
         void AddCredential(Credential credential);
 
         /// <summary>
@@ -55,23 +63,29 @@ public interface IHasVirtualAuthenticator
         /// Removes a credential from the virtual authenticator.
         /// </summary>
         /// <param name="credentialId">A byte array representing the ID of the credential to remove.</param>
+        /// <exception cref="ArgumentNullException">If <paramref name="credentialId"/> is <see langword="null"/>.</exception>
+        /// <exception cref="InvalidOperationException">If a Virtual Authenticator has not been added yet.</exception> 
         void RemoveCredential(byte[] credentialId);
 
         /// <summary>
         /// Removes a credential from the virtual authenticator.
         /// </summary>
         /// <param name="credentialId">A string representing the ID of the credential to remove.</param>
+        /// <exception cref="ArgumentNullException">If <paramref name="credentialId"/> is <see langword="null"/>.</exception>
+        /// <exception cref="InvalidOperationException">If a Virtual Authenticator has not been added yet.</exception>
         void RemoveCredential(string credentialId);
 
         /// <summary>
         /// Removes all credentials registered to this virtual authenticator.
         /// </summary>
+        /// <exception cref="InvalidOperationException">If a Virtual Authenticator has not been added yet.</exception>
         void RemoveAllCredentials();
 
         /// <summary>
         /// Sets whether or not a user is verified in this virtual authenticator.
         /// </summary>
         /// <param name="verified"><see langword="true"/> if the user is verified; otherwise <see langword="false"/>.</param>
+        /// <exception cref="InvalidOperationException">If a Virtual Authenticator has not been added yet.</exception>
         void SetUserVerified(bool verified);
     }
 }

dotnet/src/webdriver/VirtualAuth/VirtualAuthenticatorOptions.cs

@@ -20,6 +20,8 @@
 using System;
 using System.Collections.Generic;
 
+#nullable enable
+
 namespace OpenQA.Selenium.VirtualAuth
 {
     /// <summary>
@@ -78,10 +80,13 @@ public static class Transport
         private bool isUserVerified = false;
 
         /// <summary>
-        /// Sets the protocol the Virtual Authenticator speaks
+        /// Sets the Client to Authenticator Protocol (CTAP) this <see href="https://www.w3.org/TR/webauthn-2/#sctn-automation-virtual-authenticators">Virtual Authenticator</see> speaks.
         /// </summary>
-        /// <param name="protocol">Valid protocol value</param>
-        /// <returns>VirtualAuthenticatorOptions</returns>
+        /// <param name="protocol">The CTAP protocol identifier.</param>
+        /// <returns>This options instance for chaining.</returns>
+        /// <remarks>Valid protocols are available on the <see cref="Protocol"/> type.</remarks>
+        /// <exception cref="ArgumentException">If <paramref name="protocol"/> is not a supported protocol value.</exception>
+        /// <completionlist cref="Protocol"/>
         public VirtualAuthenticatorOptions SetProtocol(string protocol)
         {
             if (string.Equals(Protocol.CTAP2, protocol) || string.Equals(Protocol.U2F, protocol))
@@ -92,15 +97,19 @@ public VirtualAuthenticatorOptions SetProtocol(string protocol)
             else
             {
                 throw new ArgumentException("Enter a valid protocol value." +
-                "Refer to https://www.w3.org/TR/webauthn-2/#sctn-automation-virtual-authenticators for supported protocols.");
+                    "Refer to https://www.w3.org/TR/webauthn-2/#sctn-automation-virtual-authenticators for supported protocols.");
             }
         }
 
         /// <summary>
-        /// Sets the transport authenticator needs to implement to communicate with clients
+        /// Sets the <see href="https://www.w3.org/TR/webauthn-2/#enum-transport">Authenticator Transport</see> this <see href="https://www.w3.org/TR/webauthn-2/#sctn-automation-virtual-authenticators">Virtual Authenticator</see> needs to implement, to communicate with clients.
         /// </summary>
-        /// <param name="transport">Valid transport value</param>
-        /// <returns>VirtualAuthenticatorOptions</returns>
+        /// <param name="transport">Valid transport value.
+        /// </param>
+        /// <returns>This options instance for chaining.</returns>
+        /// <remarks>Valid protocols are available on the <see cref="Transport"/> type.</remarks>
+        /// <exception cref="ArgumentException">If <paramref name="transport"/> is not a supported transport value.</exception>
+        /// <completionlist cref="Transport"/>
         public VirtualAuthenticatorOptions SetTransport(string transport)
         {
             if (Transport.BLE == transport || Transport.INTERNAL == transport || Transport.NFC == transport || Transport.USB == transport)
@@ -111,60 +120,57 @@ public VirtualAuthenticatorOptions SetTransport(string transport)
             else
             {
                 throw new ArgumentException("Enter a valid transport value." +
-                "Refer to https://www.w3.org/TR/webauthn-2/#enum-transport for supported transport values.");
+                    "Refer to https://www.w3.org/TR/webauthn-2/#enum-transport for supported transport values.");
             }
         }
 
         /// <summary>
-        /// If set to true the authenticator will support client-side discoverable credentials.
-        /// Refer https://w3c.github.io/webauthn/#client-side-discoverable-credential
+        /// If set to <see langword="true"/>, the authenticator will support <see href="https://w3c.github.io/webauthn/#client-side-discoverable-credential">Client-side discoverable Credentials</see>.
         /// </summary>
-        /// <param name="hasResidentKey">boolean value to set</param>
-        /// <returns>VirtualAuthenticatorOptions</returns>
+        /// <param name="hasResidentKey">Whether authenticator will support client-side discoverable credentials.</param>
+        /// <returns>This options instance for chaining.</returns>
         public VirtualAuthenticatorOptions SetHasResidentKey(bool hasResidentKey)
         {
             this.hasResidentKey = hasResidentKey;
             return this;
         }
 
         /// <summary>
-        /// If set to true, the authenticator supports user verification.
-        /// Refer https://w3c.github.io/webauthn/#user-verification.
+        /// If set to <see langword="true"/>, the authenticator will support <see href="https://w3c.github.io/webauthn/#user-verification">User Verification</see>.
         /// </summary>
-        /// <param name="hasUserVerification">boolean value to set</param>
-        /// <returns></returns>
+        /// <param name="hasUserVerification">Whether the authenticator supports user verification.</param>
+        /// <returns>This options instance for chaining.</returns>
         public VirtualAuthenticatorOptions SetHasUserVerification(bool hasUserVerification)
         {
             this.hasUserVerification = hasUserVerification;
             return this;
         }
 
         /// <summary>
-        /// If set to true, a user consent will always be granted.
-        /// Refer https://w3c.github.io/webauthn/#user-consent
+        /// If set to <see langword="true"/>, a <see href="https://w3c.github.io/webauthn/#user-consent">User Consent</see> will always be granted.
+        /// Refer 
         /// </summary>
-        /// <param name="isUserConsenting">boolean value to set</param>
-        /// <returns>VirtualAuthenticatorOptions</returns>
+        /// <param name="isUserConsenting">Whether a user consent will always be granted.</param>
+        /// <returns>This options instance for chaining.</returns>
         public VirtualAuthenticatorOptions SetIsUserConsenting(bool isUserConsenting)
         {
             this.isUserConsenting = isUserConsenting;
             return this;
         }
 
         /// <summary>
-        /// If set to true, User Verification will always succeed.
-        /// Refer https://w3c.github.io/webauthn/#user-verification
+        /// If set to <see langword="true"/>, <see href="https://w3c.github.io/webauthn/#user-verification">User Verification</see> will always succeed.
         /// </summary>
-        /// <param name="isUserVerified">boolean value to set</param>
-        /// <returns>VirtualAuthenticatorOptions</returns>
+        /// <param name="isUserVerified">Whether User Verification will always succeed.</param>
+        /// <returns>This options instance for chaining.</returns>
         public VirtualAuthenticatorOptions SetIsUserVerified(bool isUserVerified)
         {
             this.isUserVerified = isUserVerified;
             return this;
         }
 
         /// <summary>
-        /// Serializes this set of options to a dictionary of key-value pairs.
+        /// Serializes this set of options into a dictionary of key-value pairs.
         /// </summary>
         /// <returns>The dictionary containing the values of this set of options.</returns>
         public Dictionary<string, object> ToDictionary()