fix: always true statement throws error #89
Conversation
🦋 Changeset detectedLatest commit: 5027e75 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
src/mdoc/models/device-response.ts
Outdated
| } | ||
| const hasMac = !!options.mac | ||
| const hasSignature = !!options.signature | ||
| if (hasMac === hasSignature) throw new EitherSignatureOrMacMustBeProvidedError() |
There was a problem hiding this comment.
Very elegant way to compare it, nice!
There was a problem hiding this comment.
Kind of a repeat with the next line const useSignature = !!options.signature but I thought it would be clearer that way.
There was a problem hiding this comment.
Hmm it seems the type validation fails:
Error: src/mdoc/models/device-response.ts(169,39): error TS18048: 'options.mac' is possibly 'undefined'.
Error: src/mdoc/models/device-response.ts(169,64): error TS18048: 'options.signature' is possibly 'undefined'.
Error: src/mdoc/models/device-response.ts(221,29): error TS2532: Object is possibly 'undefined'.
ELIFECYCLE Command failed with exit code 2.
Error: Process completed with exit code 2.
There was a problem hiding this comment.
Yes, weirdly it was already the case, but I've noticed it as well and will do another PR on this as there are more errors down the line anyway.
Head branch was pushed to by a user without write access
| sessionTranscript: options.sessionTranscript, | ||
| docType: document.docType, | ||
| deviceNamespaces: document.deviceSigned.deviceNamespaces, | ||
| deviceNamespaces, |
There was a problem hiding this comment.
I think this is still incorrect maybe, since we don't want to do any limit disclosure on the device namespaces (as devicenamesaces is the return value of limitDisclosureCb).
See also the 0.5 implementation: https://github.com/animo/mdoc/blob/v0.5.3-alpha-20250322020612/src/mdoc/model/device-response.ts#L353-L376
| } | ||
|
|
||
| issuerSigned.issuerNamespaces?.issuerNamespaces.set(nameSpace, issuerSignedItems) | ||
| deviceNamespaces.set(nameSpace, new DeviceSignedItems({ deviceSignedItems })) |
There was a problem hiding this comment.
I don't think we should set the device namespaces. Also not sure if we should modify the issuer signed directly (input mdoc).
If you look at the 0.5 implementaiton we don't modify the input: https://github.com/animo/mdoc/blob/v0.5.3-alpha-20250322020612/src/mdoc/model/pex-limit-disclosure.ts#L8-L27. So we sohuld update the device namespace to issuer namespaces, and create a new map/object and return that.
| const { namespace, digest } = result | ||
| const entry = deviceNamespaces.get(namespace) | ||
| if (!entry) { | ||
| issuerNamespaces.set(namespace, [digest]) |
There was a problem hiding this comment.
SAme here, device namespaces logic needs to be removed, and we only keep issuer namespaces.
Baiscally just copy the 0.5 implementation
2 participants
In the DeviceResponse model, I found an always true if statement that throws an error before allowing the creation of the response.