Sphereon-Opensource / OID4VC

OpenID for Verifiable Credentials - modules for issuers, holders and RPs
Apache License 2.0
62 stars 19 forks source link

fix: jwk thumprint using crypto.subtle #134

Closed TimoGlastra closed 1 month ago

TimoGlastra commented 1 month ago

This PR builds on top of #131 as it is dependant on the changes from dpop.

When testing DPoP in react native it was giving errors for us as crypto.subtle.digest was not defined. So this PR adds the custom hasher implementation to the issuer lib as well, and extends the usage in the RP/OP. Now when you want to use jwk thumprints (for dpop of id-token) you need to provide a hasher.

We could provide a default implementation when in Node.JS / browser and it hasn't been passed, but I didn't do that at the moment.

You can view the changes of this PR here: https://github.com/Sphereon-Opensource/OID4VC/commit/8d869eeea23f1db56b21d1b0211b8abd1f88100f

TimoGlastra commented 1 month ago

I think I'm going to abandon this approach as passing the hasher around everywhere is getting very cumbersome. I see in places sha.js is used, so I think I'll just update the usage of crypto.subtle to use sha.js.

What do you think @nklomp

nklomp commented 1 month ago

Yeah I like that approach more. I just wanted to make a comment that if we make more and more abstractions a developer needs to provide the harder it will become, so indeed then we at least need to provide either an example, a default or an extension that provides the implementation

nklomp commented 1 month ago

The other approach would be to have a service where you could register the hasher. Then it doesn't need to be passed around constantly. That is also the approach I took in the mdl lib

nklomp commented 1 month ago

Just have a singleton where you register a callback. Then in other places call into the registered callback

TimoGlastra commented 1 month ago

Superseded by #135