Closed frederikrothenberger closed 1 month ago
Hello @frederikrothenberger, yeah the Into
traits can work but can you give a general pseudocode how this will look please ?
@itsyaasir: Thanks for the feedback! I'll try a few things / investigate and will come back with a concrete suggestion this week.
@itsyaasir: Having spent some more time thinking about it (and concretely experimenting with a few ideas), I feel like I have underestimated the complexity of providing an extendable alternative to JwsAlgorithm
in a non-breaking way:
JwsAlgorithm
is used in many places an in particular in a many types that are part of the public API (like VerificationInput
).JwsVerifier
JwsVerifier
needs to be object safe because of impl JwsVerifier for Box<dyn JwsVerifier>
which makes it impossible to add any type of generics to it. This rules out any solution that employs an argument that is impl Into<...>
.It seems a little unfortunate that JwsAlgorithm
is both, deeply engrained in the library, and very inflexible with regards to changes that could be made to it.
Actually, even if it was possible to add a Custom(String)
variant to it, that would still be a breaking change (since it might break a match
statement in some client code)...
If you have any good suggestions I would be glad to hear them. 😉
So to proceed, I could either see:
JwsAlgorithm
not Copy
anymore and adds the Custom(String)
variant. To not make the feature sprawl too wide in the codebase, I would probably adapt it such that it does no longer rely on JwsAlgorithm
being Copy
.
name
would need to be adjusted to return String
instead of &'static str
.What do you think?
For v2 we should absolutely do this right! Our current scope for v2 is quite significant though, which would mean waiting for it to fix this inflexibility would likely be on the scale of months as we want to ideally not have to think about v3 any time soon. This opens the door for less ergonomic solutions like feature-gating suboptimal implementations as they only need to survive for a couple of months.
@eike-hass: Ok, so I'll do an implementation with a feature to gate the additional variant then.
Implemented
Description
We have custom signature scheme on the ICP blockchain that is very efficient to use for the smart contracts running on the ICP platform. It would be great if we could extend
identity_jose
to support customalg
values.Motivation
While
identity_jose
supports custom verification viaJwsVerifier
,JwsAlgorithm
is a hard-coded list of known values. This makes it impossible to implement aJwsVerifier
for an entirely customalg
value. The fixed nature ofJwsAlgorithm
is also a problem when (de)-serializingRequirements
alg
values.alg
valuesOpen questions (optional)
This change would be straight forward if
JwsAlgorithm
wasn'tCopy
and did not return a&'static str
as the name. Given that it is, we cannot just add aCustom(String)
variant to enum.Without breaking any existing code, we could maybe work with the
Into
trait to automatically convert arguments to a new type that does support custom values. I have to investigate how that would work for (de)serialization though.Are you planning to do it yourself in a pull request?
Yes