-
Notifications
You must be signed in to change notification settings - Fork 172
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
Support both Pydantic v1 and v2 #166
Conversation
This is another attempt at supporting both Pydantic v1 and v2, but without using the v1 module. Instead we configure WebAuthnBaseModel slightly differently based on the pydantic version. This contains a slight behavior change in that trailing = in base64 encoded values are not removed, because pydantic v2 doesn't support that. If that is a problem (I'm not familiar with the spec) I can try to see if I can remove them somehow by doing some post-processing of the serialized values provided by pydantic
705de48
to
71dec1a
Compare
You've put in a tremendous amount of work trying to make this all work, thank you again. 🙇
...so I feel bad asking this, but can you to try to solve the problem of the padding being included in base64url output? Padding is optional in base64url, and I think it's helped avoid the "base64 vs base64url" ambiguity by omitting the I tried figuring out how to do it with the current version of Pydantic. Perhaps affected instances of |
I put in a feature request for Pydantic v2 to propose the addition of a " |
I can see what's possibly. One option is of course to explicitly define these serializers for every bytes field. That's possible and no big problem, but it'd be a bit of code to maintain as you say. And for any downstream subclasses the developers would have to do the same. Another alternative is to dynamically add the field serializers. From what I can tell I'll take a look next week, I'm currently up in the mountains on a little cabin trip without my laptop ⛰️ |
Nice! An alternative is to ask for support for something like this: class MyBaseModel(BaseModel):
@field_serializer("*")
def bytes_to_base64url(self, value: Any, info: SerializerInfo):
# Check if field is bytes and if so serialize to base64url |
I got it working with removing trailing py_webauthn/webauthn/helpers/structs.py Lines 84 to 99 in ecf3446
I'm fairly certain CI should be green now, the previous failure was due to a mypy version not supported by the pydantic plugin. Locally I have green mypy and tests on both Pydantic v1 and v2 as this branch currently stands |
It was a tough decision, but I'm choosing this PR over #164 because this PR includes the use of V2 when it's present. That should better prepare this library for the eventual refactor to just Pydantic V2 support whenever that becomes viable. |
This is another attempt at supporting both Pydantic v1 and v2, but without using the v1 module. Instead we configure
WebAuthnBaseModel
slightly differently based on the pydantic version.This contains a slight behavior change in that trailing
=
in base64 encoded values are not removed, because pydantic v2 doesn't support that. If that is a problem (I'm not familiar with the spec) I can try to see if I can remove them somehow by doing some post-processing of the serialized values provided by pydantic