Conversation
…List, and CWTStatusToken classes
|
…ns objects for better clarity and maintainability
…y type and export CborStatusListOptions interface
…aims to use camelCase for consistency
…able names for clarity;
… verification methods
…pe for type definitions
… StatusArray and StatusList
…ven URI with timeout handling
… in CWTStatusToken
| @@ -0,0 +1,56 @@ | |||
| import * as zlib from 'pako' | |||
|
|
|||
| const arraySize = 1024 | |||
There was a problem hiding this comment.
Why do we hardcode it to 1024?
There was a problem hiding this comment.
In the specs there is no limit actually
| const expirationTime = claims.get(String(CwtStatusListClaims.ExpirationTime)) | ||
| if (expirationTime && typeof expirationTime === 'number' && expirationTime < Math.floor(Date.now() / 1000)) { | ||
| throw new Error('CWT status token has expired') | ||
| } |
There was a problem hiding this comment.
We shoud also check iat / nbf claims if they are valid right now? Would be good to move the iat / exp and nbf check to the CWT verification method, rather than the Status list (since it's verification that needs to happen for all CWTs).
Also some nits:
- I see
Math.floor(Date.now() / 1000)several times in the code. I usually create an utildateToSecondswhich optionally date a Date (and otherwise creates a new date). - I usually allow providing an optional
now?: Datethat we use as the current date if you want to for some reason have now be different than Date.now(), (in tests can be really useful as well).
There was a problem hiding this comment.
According to the spec these are not mandatory claims for a CWT.
| } else if (cwt instanceof Mac0) { | ||
| coseType = CoseType.Mac0 | ||
| } else { | ||
| throw new Error('Unimplemented/Unsupported CWT type') |
There was a problem hiding this comment.
| throw new Error('Unimplemented/Unsupported CWT type') | |
| throw new Error(`Unsupported CWT structure type ${coseType}. Supported values are sign1 and mac0`) |
There was a problem hiding this comment.
This will also fail for untagged Sign1 and Mac0 structures. However, I am not sure common that is.
| return false | ||
| } | ||
|
|
||
| static async fetchStatusListUri(statusListUri: string): Promise<Uint8Array> { |
There was a problem hiding this comment.
I think it would be great if:
- you can provide a custom fetcher to override the usage of this default one
- allow providing a custom timeout to override the default timeout.
e.g. you can look at sd-jwt-js as an example: https://github.com/openwallet-foundation/sd-jwt-js/blob/main/packages/sd-jwt-vc/src/sd-jwt-vc-instance.ts#L274
There was a problem hiding this comment.
Um, it's a static function itself. Not sure, what do you mean by custom fetcher because in the example you provided, it seems to be used by some other functions where it makes sense. But maybe not in our scenario atm
src/credential-status/status-list.ts
Outdated
| aggregationUri?: string | ||
| } | ||
|
|
||
| interface CborStatusList { |
There was a problem hiding this comment.
If CborStatusList is a cose structure, can you make it like is done for the rest of the structures in the library? This way, we have a really simple and consistent way of dealing with cose structures.
There was a problem hiding this comment.
@berendsliedrecht Can you please give some example. Will help to me to get more context and understand what you mean
There was a problem hiding this comment.
every file in this folder has a cose structure. https://github.com/animo/mdoc/tree/main/src/mdoc/models
| } | ||
|
|
||
| static async verifyStatusToken(options: CWTStatusTokenVerifyOptions): Promise<boolean> { | ||
| const cwt = cborDecode(options.token) as Sign1 | Mac0 |
There was a problem hiding this comment.
When we use the class like the rest of the lib, like mentioned above, we don't have to call cborDecode here and I think its <STRUCTURE>.fromEncodedStructure(<string>), which provides better support than decoding and type casting.
There was a problem hiding this comment.
But in here, options.token is a Uint8Array and Sign1.fromEncodedStructure() expect this [Uint8Array, Map<unknown, unknown>, Uint8Array | null, Uint8Array]
There was a problem hiding this comment.
can you try Sign1.decode?
There was a problem hiding this comment.
It might work, but we typically can't determine whether it's a Sign1 or a Mac0, which could cause issues during parsing. I believe the current approach is more reliable.
There was a problem hiding this comment.
Wait why can't we determine that? sign1 is tagged as 18 and mac0 is 17, i.e. something detectable that we can use to decode it.
This current way is not consistent with every other part of the library and we should always avoid that.
| } else if (cwt instanceof Mac0) { | ||
| coseType = CoseType.Mac0 | ||
| } else { | ||
| throw new Error('Unimplemented/Unsupported CWT type') |
There was a problem hiding this comment.
This will also fail for untagged Sign1 and Mac0 structures. However, I am not sure common that is.
…ken and CWT classes
…es; adjust StatusList to use updated bitsPerEntry accessor
… CwtOptions interfaces
…ns; update related methods and tests
…e utility; improve claim handling
…replace generic errors with specific ones
…eSecurityObject and IssuerSignedBuilder
…eSecurityObject; replace StatusInfoStructure with StatusInfoOptions
…s and encoding logic
…y; update issuer signed builder tests to include status token verification
3 participants
No description provided.