chore: update cose-kit #63
Conversation
|
|
||
| if (deviceAuth.deviceSignature) { | ||
| const deviceKey = await importCOSEKey(deviceKeyCoseKey); | ||
| const deviceKey = await COSEKey.import(deviceKeyCoseKey).toKeyLike(); |
There was a problem hiding this comment.
please note that toKeyLike() is still calling to jose's importJWK method:
There was a problem hiding this comment.
Yes, but it has additional logic that does not require a cose key to have 'alg' property.
Could you please review the PR and maybe take a look why a failure happens for the multipaz device signature?
I am looking at it but cannot figure it out. One thing I notice is that in previous version there is IssuerSignedDocument.issuerSigned.encodedProtectedHeaders with byte values, where now it has been changed to IssuerSignedDocument.issuerSigned.protectedHeaders with map values. Not sure if this change could be the culprit.
There was a problem hiding this comment.
but it has additional logic that does not require a cose key to have 'alg' property
Are you sure? Where? I'm seeing that the jwk.alg check is still there:
- https://github.com/jfromaniello/cose-kit/blob/afc207099f4a3fc957209faf32010bbc8c34f713/src/key/COSEKey.ts#L152-L153
- https://github.com/panva/jose/blob/4ae1005d83edc665d611f6ab13dc13fcfa84ac11/src/key/import.ts#L224
- https://github.com/panva/jose/blob/4ae1005d83edc665d611f6ab13dc13fcfa84ac11/src/lib/jwk_to_key.ts#L106-L109
There was a problem hiding this comment.
I was looking into version v4.15.9 of Jose, particularly the 'node' env, not taking into consideration that it also have 'browser'.
https://github.com/panva/jose/blob/v4.15.9/src/runtime/node/jwk_to_key.ts
When doing the cose-kit update it did not fail on the key import leading me to think that it has fixed it. However after going step back and testing against browser env, it was till same issue.
My application is also node application but because it is ESM module, it is using the browser runtime code tree. That is why it is failing for my code and not in the auth0/mdl tests.
Suggestion:
- Remove the multipaz test case from this PR. Have the PR reviewed as pure update to cose-kit without expectations of fixing the 'alg' issue
- Use the fix: missing jwk.alg #62 to discuss, find solution and fix the 'alg' issue
2 participants
This PR updates the cose-kit library in effort to find more appropriate solution for #62
The Multipaz example with device signature validation still fails, looking into what could be wrong.