Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: WebAuthn Discoverable Credential (Resident Credential) #33353

Closed
3 tasks done
ks75vl opened this issue Mar 20, 2022 · 5 comments
Closed
3 tasks done

[Bug]: WebAuthn Discoverable Credential (Resident Credential) #33353

ks75vl opened this issue Mar 20, 2022 · 5 comments
Labels
17-x-y bug 🪲 has-repro-gist Issue can be reproduced with code at https://gist.github.com/ stale

Comments

@ks75vl
Copy link

ks75vl commented Mar 20, 2022

Preflight Checklist

Electron Version

17.1.2

What operating system are you using?

Windows

Operating System Version

Windows 11 version 22H2

What arch are you using?

x64

Last Known Working Electron version

No response

Expected Behavior

After call navigation.credential.get() with the appropriate parameters for getting resident credential from security key. Built-in Windows FIDO2 client should popup to guide the user to complete the request.

Actual Behavior

An error was thrown: NotSupportedError: Resident credentials or empty 'allowCredentials' lists are not supported at this time.

Testcase Gist URL

https://gist.github.com/ks75vl/f6c49785a28f361c342ee60a56c2be38

Additional Information

#33271

@ks75vl ks75vl changed the title [Bug]: [Bug]: WebAuthn Discoverable Credential (Resident Credential) Mar 20, 2022
matthewloft added a commit to matthewloft/electron that referenced this issue Aug 15, 2022
…33353

Enables support for Webauthn discoverable credentials (aka resident
credentials). This allows users to authenticate without first having to
select or type a username.

To decide if discoverable credentials are supported, the class
'AuthenticatorCommon', in the chrome content code, indirectly calls the
method 'context::WebAuthenticationDelegate.SupportsResidentKeys(..)'.
The default implementation of this returns false, leaving it up to
specific implementations to override.

This change adds a new class 'ElectronWebAuthenticationDelegate' to
subclass 'WebAuthenticationDelegate' and override the behaviour of the
'SupportsResidentKeys' method to return true.
The implementation is copied from the Chrome browser equivalent
'ChromeWebAuthenticationDelegate', though the chrome class includes
other methods that don't seem to be required for this functionality.

The 'ElectronContentClient' class was also updated to store an instance
of 'ElectronWebAuthenticationDelegate', and to provide an accessor
method, GetWebAuthenticationDelegate().
@matthewloft
Copy link
Contributor

Support for discoverable credentials with physical authenticators is disabled by default in chromium.
While the Chrome browser enables it, Electron doesn't.
Submitted this pull request [/pull/35374] to turn it on.
Tested with a physical Yubikey on Windows, and works to create and get discoverable credentials.

MarshallOfSound pushed a commit that referenced this issue Sep 20, 2022
* fix: WebAuthn Discoverable Credential (Resident Credential) #33353

Enables support for Webauthn discoverable credentials (aka resident
credentials). This allows users to authenticate without first having to
select or type a username.

To decide if discoverable credentials are supported, the class
'AuthenticatorCommon', in the chrome content code, indirectly calls the
method 'context::WebAuthenticationDelegate.SupportsResidentKeys(..)'.
The default implementation of this returns false, leaving it up to
specific implementations to override.

This change adds a new class 'ElectronWebAuthenticationDelegate' to
subclass 'WebAuthenticationDelegate' and override the behaviour of the
'SupportsResidentKeys' method to return true.
The implementation is copied from the Chrome browser equivalent
'ChromeWebAuthenticationDelegate', though the chrome class includes
other methods that don't seem to be required for this functionality.

The 'ElectronContentClient' class was also updated to store an instance
of 'ElectronWebAuthenticationDelegate', and to provide an accessor
method, GetWebAuthenticationDelegate().

* Remove redundant, commented-out code

* style: comment cleanup

* style: updated comments and formatting based on pull request review

* style: fix lint error on header guard clause
@matthewloft
Copy link
Contributor

@ks75vl PR #35374 is now merged, so you may want to retest.
Depending on your IdP, you may need to strip out any any custom agents (e.g. "Electron/xx.xx" or "MyAppName/xx.xx") from the User Agent string. IdP's can suppress security key functionality based on what they see in the User Agent. Azure AD certainly does this. Not sure about others.

@ckerr ckerr added 17-x-y has-repro-gist Issue can be reproduced with code at https://gist.github.com/ labels Sep 21, 2022
@ruiquelhas
Copy link

ruiquelhas commented Oct 21, 2022

I'm assuming this feature has been introduced in electron@22.0.0-alpha.1 according to the release notes but I've tried it using electron@22.0.0-alpha.7 and it still does not seem to work as expected. In this case, if the allowCredentials property is not defined or is an empty list, instead of the error reported by OP, it yields:

The operation either timed out or was not allowed. See: https://www.w3.org/TR/webauthn-2/#sctn-privacy-considerations-client.

If allowCredentials contains a valid credentialId, everything works fine. So, I'm guessing there is still something missing.

@github-actions
Copy link
Contributor

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. If you have any new additional information—in particular, if this is still reproducible in the latest version of Electron or in the beta—please include it with your comment!

@github-actions github-actions bot added the stale label Jan 20, 2023
@github-actions
Copy link
Contributor

This issue has been closed due to inactivity, and will not be monitored. If this is a bug and you can reproduce this issue on a supported version of Electron please open a new issue and include instructions for reproducing the issue.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Feb 19, 2023
khalwa pushed a commit to solarwindscloud/electron that referenced this issue Feb 22, 2023
…#35374)

* fix: WebAuthn Discoverable Credential (Resident Credential) electron#33353

Enables support for Webauthn discoverable credentials (aka resident
credentials). This allows users to authenticate without first having to
select or type a username.

To decide if discoverable credentials are supported, the class
'AuthenticatorCommon', in the chrome content code, indirectly calls the
method 'context::WebAuthenticationDelegate.SupportsResidentKeys(..)'.
The default implementation of this returns false, leaving it up to
specific implementations to override.

This change adds a new class 'ElectronWebAuthenticationDelegate' to
subclass 'WebAuthenticationDelegate' and override the behaviour of the
'SupportsResidentKeys' method to return true.
The implementation is copied from the Chrome browser equivalent
'ChromeWebAuthenticationDelegate', though the chrome class includes
other methods that don't seem to be required for this functionality.

The 'ElectronContentClient' class was also updated to store an instance
of 'ElectronWebAuthenticationDelegate', and to provide an accessor
method, GetWebAuthenticationDelegate().

* Remove redundant, commented-out code

* style: comment cleanup

* style: updated comments and formatting based on pull request review

* style: fix lint error on header guard clause
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
17-x-y bug 🪲 has-repro-gist Issue can be reproduced with code at https://gist.github.com/ stale
Projects
None yet
Development

No branches or pull requests

4 participants