marcpfuller commented 2025年11月26日 03:09:25 +01:00

When MFA_PASSKEY_LOGIN_ENABLED is True, users were able to authenticate
with any WebAuthn credential regardless of the individual credential's
passwordless field setting. The passwordless checkbox on the credential
creation form had no effect on authentication behavior.

Changes:

• Modified auth.get_credentials() to accept passwordless_only parameter
  that filters credentials based on their passwordless field
• Modified auth.begin_authentication() and auth.complete_authentication()
  to pass passwordless flag through the authentication flow
• Updated flows.add_authenticator() to save the passwordless flag when
  creating new credentials
• Modified forms.AddWebAuthnForm to include passwordless checkbox with
  help text explaining its purpose
• Updated forms.AuthenticateWebAuthnForm to pass passwordless flag to
  auth.complete_authentication()
• Modified forms.LoginWebAuthnForm to set passwordless=True for
  passwordless login flows
• Updated views.AddWebAuthnView to extract and pass passwordless flag
  when adding credentials
• Modified views.LoginWebAuthnView to pass passwordless=True to
  auth.begin_authentication() and use flows.perform_passwordless_login()
• Added comprehensive unit tests to verify filtering behavior

The fix ensures that:

• Passwordless authentication (LoginWebAuthnView) only uses credentials
  with passwordless=True
• MFA authentication (ReauthenticateWebAuthnView) uses all WebAuthn
  credentials regardless of passwordless flag
• Credentials without explicit passwordless field default to False
• The global MFA_PASSKEY_LOGIN_ENABLED setting still controls feature
  availability

Fixes: #4599

Signed-off-by: marc fuller gogita99@gmail.com

Submitting Pull Requests

General

• Make sure you use semantic commit messages.
  Examples: "fix(google): Fixed foobar bug", "feat(accounts): Added foobar feature".
• All Python code must formatted using Black, and clean from pep8 and isort issues.
• JavaScript code should adhere to StandardJS.
• If your changes are significant, please update ChangeLog.rst.
• If your change is substantial, feel free to add yourself to AUTHORS.

Provider Specifics

In case you add a new provider:

• Make sure unit tests are available.
• Ensure your provider is tested by adding an entry over at tests/projects/common/settings.py::INSTALLED_SOCIALACCOUNT_APPS.
• Provide provider specific documentation at docs/socialaccount/providers/<provider name>.rst.
• Link to your provider specific documentation at docs/insocialaccount/providers/index.rst.
• Add an entry for your provider in the quickstart, over at docs/installation/quickstart.rst::INSTALLED_APPS.

When MFA_PASSKEY_LOGIN_ENABLED is True, users were able to authenticate with any WebAuthn credential regardless of the individual credential's passwordless field setting. The passwordless checkbox on the credential creation form had no effect on authentication behavior. Changes: - Modified auth.get_credentials() to accept passwordless_only parameter that filters credentials based on their passwordless field - Modified auth.begin_authentication() and auth.complete_authentication() to pass passwordless flag through the authentication flow - Updated flows.add_authenticator() to save the passwordless flag when creating new credentials - Modified forms.AddWebAuthnForm to include passwordless checkbox with help text explaining its purpose - Updated forms.AuthenticateWebAuthnForm to pass passwordless flag to auth.complete_authentication() - Modified forms.LoginWebAuthnForm to set passwordless=True for passwordless login flows - Updated views.AddWebAuthnView to extract and pass passwordless flag when adding credentials - Modified views.LoginWebAuthnView to pass passwordless=True to auth.begin_authentication() and use flows.perform_passwordless_login() - Added comprehensive unit tests to verify filtering behavior The fix ensures that: - Passwordless authentication (LoginWebAuthnView) only uses credentials with passwordless=True - MFA authentication (ReauthenticateWebAuthnView) uses all WebAuthn credentials regardless of passwordless flag - Credentials without explicit passwordless field default to False - The global MFA_PASSKEY_LOGIN_ENABLED setting still controls feature availability Fixes: #4599 Signed-off-by: marc fuller <gogita99@gmail.com> # Submitting Pull Requests ## General - [x] Make sure you use [semantic commit messages](https://seesparkbox.com/foundry/semantic_commit_messages). Examples: `"fix(google): Fixed foobar bug"`, `"feat(accounts): Added foobar feature"`. - [x] All Python code must formatted using Black, and clean from pep8 and isort issues. - [x] JavaScript code should adhere to [StandardJS](https://standardjs.com). - [ ] If your changes are significant, please update `ChangeLog.rst`. - [ ] If your change is substantial, feel free to add yourself to `AUTHORS`. ## Provider Specifics In case you add a new provider: - [ ] Make sure unit tests are available. - [ ] Ensure your provider is tested by adding an entry over at `tests/projects/common/settings.py::INSTALLED_SOCIALACCOUNT_APPS`. - [ ] Provide provider specific documentation at `docs/socialaccount/providers/<provider name>.rst`. - [ ] Link to your provider specific documentation at `docs/insocialaccount/providers/index.rst`. - [ ] Add an entry for your provider in the quickstart, over at `docs/installation/quickstart.rst::INSTALLED_APPS`.

pennersr commented 2025年11月26日 20:48:48 +01:00

Last time I checked it was not possible to reliably check whether or not a credential is passwordless. That is why this method can return None:

 @property
 def is_passwordless(self) -> Optional[bool]:
 return (
 self.instance.data.get("credential", {})
 .get("clientExtensionResults", {})
 .get("credProps", {})
 .get("rk")
 )

As you see, in order to tell if a credential is passwordless you have to rely on an extension which not all authenticators support. If we cannot tell if a credential can truly operate in a passwordless fashion, that is problematic, as even if the user checks the [X] Passwordless option it is not clear that is truly what the user is getting.

In this PR you're marking credentials as passwordless based on the checkbox but we likely need to keep the above into account as well.

Last time I checked it was not possible to reliably check whether or not a credential is passwordless. That is why this method can return `None`: ``` @property def is_passwordless(self) -> Optional[bool]: return ( self.instance.data.get("credential", {}) .get("clientExtensionResults", {}) .get("credProps", {}) .get("rk") ) ``` As you see, in order to tell if a credential is passwordless you have to rely on an extension which not all authenticators support. If we cannot tell if a credential can truly operate in a passwordless fashion, that is problematic, as even if the user checks the `[X] Passwordless` option it is not clear that is truly what the user is getting. In this PR you're marking credentials as passwordless based on the checkbox but we likely need to keep the above into account as well.

pennersr commented 2025年11月26日 20:51:37 +01:00

Doesn't this break backwards compatibility? Passkeys created by older allauth versions don't have the passwordless property stored.

Doesn't this break backwards compatibility? Passkeys created by older allauth versions don't have the `passwordless` property stored.

marcpfuller commented 2025年11月27日 02:46:15 +01:00

@pennersr wrote in #4615 (comment):

Last time I checked it was not possible to reliably check whether or not a credential is passwordless. That is why this method can return None:

 @property
 def is_passwordless(self) -> Optional[bool]:
 return (
 self.instance.data.get("credential", {})
 .get("clientExtensionResults", {})
 .get("credProps", {})
 .get("rk")
 )

As you see, in order to tell if a credential is passwordless you have to rely on an extension which not all authenticators support. If we cannot tell if a credential can truly operate in a passwordless fashion, that is problematic, as even if the user checks the [X] Passwordless option it is not clear that is truly what the user is getting.

In this PR you're marking credentials as passwordless based on the checkbox but we likely need to keep the above into account as well.

@pennersr, I've updated the implementation to verify actual passwordless capability using credProps.rk instead of trusting the checkbox.

Key changes:

[x] complete_registration() now returns the actual capability from the authenticator
[x] Form validation rejects registration if capability is False or None (unknown)
[x] Only stores passwordless: true when explicitly verified as True
[x] Added backwards compatibility for legacy credentials via auto-migration
[x] This ensures we never falsely claim a credential is passwordless when we can't verify it. The implementation errs on the side of caution - if capability can't be confirmed, passwordless is disabled.

@pennersr wrote in https://codeberg.org/allauth/django-allauth/pulls/4615#issuecomment-8509944: > Last time I checked it was not possible to reliably check whether or not a credential is passwordless. That is why this method can return `None`: > > ```text > @property > def is_passwordless(self) -> Optional[bool]: > return ( > self.instance.data.get("credential", {}) > .get("clientExtensionResults", {}) > .get("credProps", {}) > .get("rk") > ) > ``` > > As you see, in order to tell if a credential is passwordless you have to rely on an extension which not all authenticators support. If we cannot tell if a credential can truly operate in a passwordless fashion, that is problematic, as even if the user checks the `[X] Passwordless` option it is not clear that is truly what the user is getting. > > In this PR you're marking credentials as passwordless based on the checkbox but we likely need to keep the above into account as well. @pennersr, I've updated the implementation to verify actual passwordless capability using credProps.rk instead of trusting the checkbox. Key changes: [x] complete_registration() now returns the actual capability from the authenticator [x] Form validation rejects registration if capability is False or None (unknown) [x] Only stores passwordless: true when explicitly verified as True [x] Added backwards compatibility for legacy credentials via auto-migration [x] This ensures we never falsely claim a credential is passwordless when we can't verify it. The implementation errs on the side of caution - if capability can't be confirmed, passwordless is disabled.

pennersr commented 2025年12月14日 14:20:25 +01:00

[x] Form validation rejects registration if capability is False or None (unknown)

Here we are potentially locking out users. Without these changes, an authenticator that does not support the extension would return None, yet, would possibly still be usable in a passwordless fashion. With these changes that is no longer possible.

The passwordless checkbox on the credential creation form had no effect on authentication behavior.

Currently, the semantics of that checkbox is that it only influences the capabilities of the credential that is being requested. It indeed does not pose any limitations on whether or not that credential can be allowed to login with. Your PR changes the semantics, and the question is whether this new semantics should replace the current semantics, or, whether it is something to be opted into. I am also wondering if this passwordless toggle should be editable?

In any case, before proceeding with this, I would like to see how other parties are doing this. It would be good to have an overview of say GitHub, Sentry and a few big others to see how they are approaching this. Perhaps you have done some research already in this regard?

> [x] Form validation rejects registration if capability is False or None (unknown) Here we are potentially locking out users. Without these changes, an authenticator that does not support the extension would return `None`, yet, would possibly still be usable in a passwordless fashion. With these changes that is no longer possible. > The passwordless checkbox on the credential creation form had no effect on authentication behavior. Currently, the semantics of that checkbox is that it only influences the capabilities of the credential that is being requested. It indeed does not pose any limitations on whether or not that credential can be allowed to login with. Your PR changes the semantics, and the question is whether this new semantics should replace the current semantics, or, whether it is something to be opted into. I am also wondering if this passwordless toggle should be editable? In any case, before proceeding with this, I would like to see how other parties are doing this. It would be good to have an overview of say GitHub, Sentry and a few big others to see how they are approaching this. Perhaps you have done some research already in this regard?

This pull request has changes conflicting with the target branch.