[py] Fix default rpId in virtual authenticator

[cgoldberg]

User description

🔗 Related Issues

Fixes #16424

💥 What does this PR do?

This PR sets rp_id to None in the Credential created by Credential.from_dict() if the rpId key is missing. This was inadvertently changed in #16174, causing a breaking change.

🔄 Types of changes

• Bug fix (backwards compatible)

---

PR Type

Bug fix

---

Description

• Fix Credential.from_dict() to handle missing rpId key gracefully

• Restore default rp_id to None when rpId is absent from input dictionary

• Resolve regression introduced in previous virtual authenticator changes

---

Diagram Walkthrough

flowchart LR
  A["Credential.from_dict()"] --> B["Check rpId in data"]
  B --> C["Use data.get() with None default"]
  C --> D["Return Credential with rp_id=None if missing"]

Loading

File Walkthrough

	Relevant files
Bug fix	virtual_authenticator.py Use safe dictionary access for rpId field                                py/selenium/webdriver/common/virtual_authenticator.py Changed rpId extraction from direct dictionary access to .get() method Set default value to None when rpId key is missing Aligns with existing pattern used for userHandle field +1/-1

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
⚪	No custom compliance provided Follow the guide to enable custom compliance check.
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Latest suggestions up to 8899e13

Category	Suggestion	Impact
Incremental [*]	Validate and default missing rp_id In the init method, validate that rp_id is provided for non-resident credentials and default it to an empty string if it is None to ensure type consistency. py/selenium/webdriver/common/virtual_authenticator.py [79-104] def __init__( self, credential_id: bytes, is_resident_credential: bool, rp_id: Optional[str], user_handle: Optional[bytes], private_key: bytes, sign_count: int, ): """Constructor. A credential stored in a virtual authenticator. https://w3c.github.io/webauthn/#credential-parameters. :Args: - credential_id (bytes): Unique base64 encoded string. - is_resident_credential (bool): Whether the credential is client-side discoverable. - - rp_id (str): Relying party identifier. + - rp_id (str | None): Relying party identifier. Optional for resident credentials. - user_handle (bytes): userHandle associated to the credential. Must be Base64 encoded string. Can be None. - private_key (bytes): Base64 encoded PKCS#8 private key. - sign_count (int): initial value for a signature counter. """ self._id = credential_id self._is_resident_credential = is_resident_credential - self._rp_id = rp_id + # Default to empty string if rp_id is None to keep property type stable + # Alternatively, enforce presence for non-resident credentials + if not is_resident_credential and rp_id is None: + raise ValueError("rp_id is required for non-resident credentials") + self._rp_id = rp_id or "" self._user_handle = user_handle self._private_key = private_key self._sign_count = sign_count [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that allowing rp_id to be None introduces a type inconsistency with the rp_id property which is hinted to return str, and proposes a robust fix in the __init__ method to prevent this invalid state.	Medium
Possible issue	Default missing rpId on parse In Credential.from_dict, default a missing rpId to an empty string instead of None to prevent passing None to the constructor. py/selenium/webdriver/common/virtual_authenticator.py [179-188] @classmethod def from_dict(cls, data: dict[str, Any]) -> "Credential": _id = urlsafe_b64decode(f"{data['credentialId']}==") is_resident_credential = bool(data["isResidentCredential"]) - rp_id = data.get("rpId", None) + rp_id_val = data.get("rpId") or "" private_key = urlsafe_b64decode(f"{data['privateKey']}==") sign_count = int(data["signCount"]) - user_handle = urlsafe_b64decode(f"{data['userHandle']}==") if data.get("userHandle", None) else None + user_handle = urlsafe_b64decode(f"{data['userHandle']}==") if data.get("userHandle") else None - return cls(_id, is_resident_credential, rp_id, user_handle, private_key, sign_count) + return cls(_id, is_resident_credential, rp_id_val, user_handle, private_key, sign_count) [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 6 __ Why: This suggestion correctly points out that from_dict can pass None for rp_id to the constructor. While this is a valid concern, the primary fix should be in the __init__ method to handle the None case centrally, as suggested by another suggestion. This change is beneficial but somewhat redundant if the constructor is fixed.	Low
Learned best practice	Align docstring with optional rp_id Update the docstring to reflect that rp_id can be optional and document when it may be None to keep the API contract accurate. py/selenium/webdriver/common/virtual_authenticator.py [79-98] def __init__( self, credential_id: bytes, is_resident_credential: bool, rp_id: Optional[str], user_handle: Optional[bytes], private_key: bytes, sign_count: int, ): """Constructor. A credential stored in a virtual authenticator. https://w3c.github.io/webauthn/#credential-parameters. :Args: - credential_id (bytes): Unique base64 encoded string. - is_resident_credential (bool): Whether the credential is client-side discoverable. - - rp_id (str): Relying party identifier. - - user_handle (bytes): userHandle associated to the credential. Must be Base64 encoded string. Can be None. + - rp_id (Optional[str]): Relying party identifier. May be None in scenarios where RP ID is not applicable. + - user_handle (Optional[bytes]): userHandle associated to the credential. Must be Base64 encoded string. Can be None. - private_key (bytes): Base64 encoded PKCS#8 private key. - - sign_count (int): initial value for a signature counter. + - sign_count (int): Initial value for a signature counter. """ [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Keep API and documentation accurate and consistent with parameter types and behavior.	Low
General	Make string output robust In Credential.str, handle the case where rp_id is None to ensure the string representation is always robust and does not output "None". py/selenium/webdriver/common/virtual_authenticator.py [190-192] def __str__(self) -> str: - return f"Credential(id={self.id}, is_resident_credential={self.is_resident_credential}, rp_id={self.rp_id},\ - user_handle={self.user_handle}, private_key={self.private_key}, sign_count={self.sign_count})" + rp = self.rp_id if self.rp_id is not None else "" + return ( + f"Credential(id={self.id}, is_resident_credential={self.is_resident_credential}, " + f"rp_id={rp}, user_handle={self.user_handle}, private_key={self.private_key}, " + f"sign_count={self.sign_count})" + ) [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 4 __ Why: The suggestion correctly notes that rp_id could be None in the __str__ output. However, if the __init__ method is fixed to normalize None to an empty string, this change becomes unnecessary as self.rp_id would never be None. It's a valid point but addresses a symptom rather than the root cause.	Low
More

---

Previous suggestions

Suggestions up to commit c56ad9a

Category	Suggestion	Impact
Possible issue	Revert change to prevent type mismatch Revert the use of data.get("rpId", None) to data["rpId"]. The Credential constructor expects rp_id to be a str, but the change allows it to be None, which would cause a runtime error. py/selenium/webdriver/common/virtual_authenticator.py [183] - rp_id = data["rpId"] -+ rp_id = data.get("rpId", None) ++ rp_id = data["rpId"] Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a bug introduced by the PR, where passing None as rp_id to the Credential constructor would cause a TypeError due to a type hint mismatch.	High

[navin772]

LGTM! Should we have a test to prevent this in future?

[cgoldberg]

the RBE failure seems unrelated.