[dotnet] Fix webauth credential to allow nullable rpID

[nvborisenko]

User description

Motivation and Context

Fixes issue, introduced in recent versions.

https://github.com/SeleniumHQ/seleniumhq.github.io/actions/runs/13062596341/job/36448769281

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

---

PR Type

Bug fix

---

Description

• Made rpId parameter nullable in Credential class.

• Updated constructors and methods to handle nullable rpId.

• Adjusted exception documentation to reflect nullable rpId.

---

Changes walkthrough 📝

	Relevant files
Bug fix	Credential.cs Allow nullable `rpId` in `Credential` class                            dotnet/src/webdriver/VirtualAuth/Credential.cs Changed rpId parameter to nullable in constructor. Updated CreateNonResidentCredential and CreateResidentCredential methods to accept nullable rpId. Adjusted exception documentation for rpId parameter. +6/-6

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 Security concerns Authentication Validation: The PR makes the relying party ID (rpId) parameter nullable without documenting the security implications. The rpId is typically used for domain validation in WebAuthn/FIDO authentication. Making it nullable could potentially allow credentials to be used across different domains if not properly validated elsewhere in the codebase.

⚡ Recommended focus areas for review Validation Check The PR removes rpId validation but doesn't document why rpId is now allowed to be null or what the implications are for credential security and functionality private Credential(byte[] id, bool isResidentCredential, string? rpId, string privateKey, byte[]? userHandle, int signCount) { 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; }

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	Add missing parameter validation Add validation for userHandle parameter in CreateResidentCredential method since it's a required parameter for resident credentials according to WebAuthn specification. dotnet/src/webdriver/VirtualAuth/Credential.cs [71-74] public static Credential CreateResidentCredential(byte[] id, string? rpId, string privateKey, byte[] userHandle, int signCount) { + if (userHandle == null) throw new ArgumentNullException(nameof(userHandle)); return new Credential(id, true, rpId, privateKey, userHandle, signCount); } Apply this suggestion Suggestion importance[1-10]: 9 Why: The suggestion addresses a critical validation gap for resident credentials where userHandle is required by WebAuthn spec but currently not validated, which could lead to runtime issues. Adding this null check is essential for maintaining data integrity and spec compliance.	9

[RenderMichael]

I could be misremembering, but I think a credential doesn't work if the rpId is null.

I'm a little concerned about misleading users if we mark the field as nullable. In order to get rid of the null argument warning, maybe we can change FromDictionary from TryGetValue to a direct indexer.

[nvborisenko]

Found this one: https://chromedevtools.github.io/devtools-protocol/tot/WebAuthn/#type-Credential

Seems rpId is optional, but user must set it when adding new credential. If user will get exception from remote end, it is totally fine.

[RenderMichael]

What if the Create{Non}ResidentCredential methods had a non-nullable rpid parameter, but the constructor and RpId property allowed null? That way, we will warn users about setting the value, but we will not throw any exceptions ourselves.

[RenderMichael]

Per comments about rpId being optional, this change makes sense to me

[RenderMichael]

Nice! These changes offers nullability guidance to users without throwing any exceptions on the C# side