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

Missing initializers for some public objects. #74

Open
mikeshappell opened this issue Jul 24, 2024 · 0 comments
Open

Missing initializers for some public objects. #74

mikeshappell opened this issue Jul 24, 2024 · 0 comments

Comments

@mikeshappell
Copy link

mikeshappell commented Jul 24, 2024

Good morning everyone.

While working on a project using the Swift WebAuthn library, I noticed that a number of public objects don't have public or default initializers. I am not sure if this was a intentional decision or an oversight. However, before I head down a path that might not make sense, I wanted to reach out to understand the thinking here.

Summary

As the WebAuthn package is a server-focused solution, I have elected to utilize DTOs that are shared between the client and service to facilitate their communication. This servers a couple of purposes, but the primary one is to decouple the service and application from the implementation details of a third-party library. While these public definitions may not change much or ever, I prefer to define a solution unencumbered by this fact.

Details

That means that on the server, when the DTO is decoded, it must be "converted" to the WebAuthn type that is needed. Generally, this involves creating a new instance of the object using the default initializer or convenence initializer where that makes sense.

However, as a number of the public objects (e.g., RegistrationCredential) don't have public initializers, that is not possible. The only initializer that is available at the moment is for a decoder (not even an encoder). Therefore, to accomplish the goal, one of two things must happen.

Option 1: Make the DTO the exact same as the public object available in WebAuthn and just decode it directly. Of course this does not allow the DTO to deviate in any way from the public object (not always desirable) and keeps the two tightly coupled.

Option 2: Create different encoders for the application (actually the synthesized encoder works here) and one on the server that encodes to the format required by the public object. Then, that data can be decoded using the initializer on the object to create an instance. Obviously, not an ideal workaround, but also complicates the code as two different encoders are required by the same object. This requires different libraries in a shared package to ensure that each side picks up the right encoder.

Recommended Solution

Add default, public initializers to every public object. I am not seeing any harm in doing this as these objects are public. Further, I am not seeing any downsides to being able to instantiate a copy of any of these objects outside of the specific need of the package. With that said, I have not spent enough time analyzing the package to know that these statements are 100% true. Therefore, I am interested in the choice that was made here. My inclination is to add the initializers and submit a pull request, but I also don't like wasting time if this was an intentional decision by the author.

@mikeshappell mikeshappell changed the title Missing initializers for some of the structs... Missing initializers for some public objects. Jul 25, 2024
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

No branches or pull requests

1 participant