Skip to content

Conversation

maartenba
Copy link

@maartenba maartenba commented Jul 25, 2025

Passkeys - JSON.stringify() fallback for various password managers

Some password managers do not implement PublicKeyCredential.prototype.toJSON correctly, which is required for JSON.stringify() to work when serializing a passkey credential - e.g. https://www.1password.community/discussions/1password/typeerror-illegal-invocation-in-chrome-browser/47399

This PR adds a fallback to work with various password managers.

@Copilot Copilot AI review requested due to automatic review settings July 25, 2025 05:31
@github-actions github-actions bot added the area-blazor Includes: Blazor, Razor Components label Jul 25, 2025
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jul 25, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a fallback mechanism for JSON serialization of passkey credentials to handle password managers that don't properly implement PublicKeyCredential.prototype.toJSON. The change addresses compatibility issues where JSON.stringify() fails with an "Illegal invocation" error in certain password managers like 1Password.

  • Wraps the existing JSON.stringify(credential) call in a try-catch block
  • Implements manual JSON serialization as a fallback when the standard approach fails
  • Adds a helper function to convert ArrayBuffer/Uint8Array data to base64url format

maartenba and others added 3 commits July 25, 2025 07:34
…harp/BlazorWebCSharp.1/Components/Account/Shared/PasskeySubmit.razor.js

Co-authored-by: Copilot <[email protected]>
…harp/BlazorWebCSharp.1/Components/Account/Shared/PasskeySubmit.razor.js

Co-authored-by: Copilot <[email protected]>
…harp/BlazorWebCSharp.1/Components/Account/Shared/PasskeySubmit.razor.js

Co-authored-by: Copilot <[email protected]>
@abergs
Copy link

abergs commented Jul 25, 2025

Since I was pinged, for some long forgotten reason I rewrote the helpers here:
https://github.com/bitwarden/passwordless-client-js/blob/main/src/passwordless.ts#L439-L487 (Apache 2.0)

…harp/BlazorWebCSharp.1/Components/Account/Shared/PasskeySubmit.razor.js
@javiercn javiercn requested a review from MackinnonBuck July 30, 2025 10:45
@javiercn javiercn added the area-identity Includes: Identity and providers label Jul 30, 2025
@javiercn
Copy link
Member

@maartenba can you name which password managers are affected by this?

I'm not keen on dumping a bunch of code like this on the template to work around a bug in a password manager implementation. I would rather have the password managers fix their implementation to be compliant.

As an alternative we can offer this as a sample but putting it on the template raises other concerns like maintainability, updates and so on in the longer term.

@maartenba
Copy link
Author

@javiercn agree this is the password manager's responsibility, looks like at least 1Password has had this issue for a longer time now with no resolution in sight. If there are docs (or planned docs) I'm happy to suggest a PR there to provide guidance to others who may want to handle this issue in their application.

@javiercn
Copy link
Member

@mikekistler @MackinnonBuck thoughts on putting this on docs instead?

@mikekistler
Copy link
Contributor

@javiercn @maartenba I agree that we probably don't want to bloat the templates with workaround logic for bugs in password managers. We are certainly planning docs for the Passkey support and putting some guidance there for this issue seems reasonable.

@maartenba
Copy link
Author

Feel free to reuse the JS code here, or if you'd like that as a contribution when working on docs I'd be happy to help.

@MackinnonBuck
Copy link
Member

I agree that mentioning this in the docs is the right direction.

@javiercn
Copy link
Member

javiercn commented Aug 3, 2025

@maartenba @MackinnonBuck @mikekistler I've created #63078 to track the doc additions. I'm going to close the PR for now since we don't plan to take it.

@maartenba thanks in any case for putting this forward and starting the discussion.

@javiercn javiercn closed this Aug 3, 2025
@dotnet-policy-service dotnet-policy-service bot added this to the 10.0-rc1 milestone Aug 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-blazor Includes: Blazor, Razor Components area-identity Includes: Identity and providers community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants