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

✨ Added support for py webauthn 2.0+ #702

Merged
merged 2 commits into from
Feb 3, 2024
Merged

Conversation

sergei-maertens
Copy link
Contributor

@sergei-maertens sergei-maertens commented Feb 2, 2024

Description

Closes #701

WebAuthn 2.0 refactored pydantic usage out of the codebase.

Tests were passing locally with both 1.x and 2.y, but for simplicity's sake, the minimum version is now set to 2.0 so that no compat layer is required.

There doesn't seem to have been a real reason to catch pydantic validation errors - nothing was documented in 96bbd1a and the model does not appear to perform additional validations (see
https://github.com/duo-labs/py_webauthn/blob/v1.11.1/webauthn/authentication/verify_authentication_response.py ), but possibly the base model did. Either way, it was not a tested execution branch.

Motivation and Context

Drops dependency on Pydantic which may cause version conflicts with other tools.

How Has This Been Tested?

tox -e py311-dj42-webauthn and having the Github CI pipeline. There shouldn't be any functional changes.

Screenshots (if appropriate):

n/a

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • n/a I have added tests to cover my changes.
  • All new and existing tests passed. (waiting for CI to run the rest)

@sergei-maertens
Copy link
Contributor Author

I will investigate the issues on Python < 3.11!

Copy link

codecov bot commented Feb 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (ca8de19) 97.83% compared to head (8fc2055) 97.83%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #702      +/-   ##
==========================================
- Coverage   97.83%   97.83%   -0.01%     
==========================================
  Files          78       78              
  Lines        3379     3378       -1     
  Branches      376      376              
==========================================
- Hits         3306     3305       -1     
  Misses         42       42              
  Partials       31       31              

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

@sergei-maertens sergei-maertens marked this pull request as ready for review February 3, 2024 15:59
@sergei-maertens
Copy link
Contributor Author

sergei-maertens commented Feb 3, 2024

@claudep seems to be a pretty straight-forward upgrade

I CTRL + F the docs to see if there was anything to update, but I couldn't find anything. A mention in the changelog is probably sufficient?

Let me know if you prefer to keep the invidivual commits or would rather see them squashed, and if #700 lands first, I can obviously rebase.

@claudep
Copy link
Contributor

claudep commented Feb 3, 2024

May I ask you to approve my recent PRs, then you can rebase yours on top?

@sergei-maertens
Copy link
Contributor Author

Of course, I was already taking a look out of interest :)

WebAuthn 2.0 refactored pydantic usage out of the codebase.

For simplicity's sake, the minimum version is now set to 2.0
so that no compat layer is required.

It appears that wat used to be Pydantic validation errors are
now raised as InvalidJSONStructure exceptions, the form validation
code is updated to reflect that.
This has changed in webauthn 2.0+ compared to 1.x
Copy link
Contributor

@claudep claudep left a comment

Choose a reason for hiding this comment

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

Thanks!

@claudep claudep merged commit 6f3bbd8 into master Feb 3, 2024
13 checks passed
@claudep claudep deleted the feature/701-webauthn-2.0 branch February 3, 2024 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

What is needed to upgrade to webauthn 2.0.0+?
2 participants