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

Introduce WebAuthnUserHandle to persist a truly random user handle #44

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Stormheg
Copy link
Member

@Stormheg Stormheg commented Dec 1, 2024

Replaces the previous approach of hashing the users' primary key and implements the guidance in https://www.w3.org/TR/webauthn-3/#sctn-user-handle-privacy

This yields better privacy, as there is no way now to link a user handle back to a user account. See the security note about a possible de-anonymization attack vector removed by this commit.

When I originally wrote the handle generation code and said security note, I was under the impression that storing random bytes would have to be done on the user model. Requiring developers to implement a WebAuthnUserHandle mixin or the like in their custom user model to add this field. That would be more steps and add unwanted bloat.

With the OneToOneField approach it works more transparently, with no effort required from the developer. Wish I thought of this from the beginning.

Replaces the previous approach of hashing the users' primary key and
implements the guidance in https://www.w3.org/TR/webauthn-3/#sctn-user-handle-privacy

This yields better privacy, as there is no way now to link a user
handle back to a user account. See the security note about a possible
de-anonymization attack vector removed by this commit.

When I originally wrote the handle generation code and said security
note, I was under the impression that storing random bytes would have to
be done on the user model. Requiring developers to implement a
`WebAuthnUserHandle` mixin or the like in their custom user model to add
this field. That would be more steps and add unwanted bloat.

With the OneToOneField approach it works more transparently, with no
effort required from the developer. Wish I thought of this from the
beginning.
Copy link

codecov bot commented Dec 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.0%. Comparing base (e32901c) to head (17998c0).
Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #44   +/-   ##
=======================================
  Coverage   100.0%   100.0%           
=======================================
  Files          13       13           
  Lines         683      709   +26     
  Branches       56       57    +1     
=======================================
+ Hits          683      709   +26     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Stormheg Stormheg force-pushed the feature/improve-user-handle-generation branch from bc6037c to 17998c0 Compare December 1, 2024 17:54
@Stormheg Stormheg marked this pull request as draft December 1, 2024 23:10
@Stormheg
Copy link
Member Author

Need to figure out how this affects custom credential models and possibly adjust instructions to take into account a manual migration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant