Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: check vct scheme to choose hardware/software key #133

Merged
merged 1 commit into from
Aug 14, 2024

Conversation

berendsliedrecht
Copy link
Member

Signed-off-by: Berend Sliedrecht [email protected]

})
}
// TODO: require support for mDoc namespace here as well
const vct = resolvedCredentialOffer.offeredCredentialConfigurations[offeredCredentialToRequest.id].vct
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use supportedCredentialId that is passed to this callback. This will help us when we add mdoc as well and request multiple creds.

Also -- vct is not always defined. So this check should be something like:

let shouldKeyBeHardwareBacked = false

const offeredCredentialConfiguration = offeredCredentialConfigurations[supportedCredentialId]

// TODO: use the enum for format, not sure what its' called)
if (offeredCredentialConfiguration.format === 'vc+sd-jwt' && pidSchemes?.sdJwtVcVcts.includes(offeredCredentialConfiguration.vct)) {
  shouldKeyBeHardwareBacked = true
}

Finally, I'm not sure why we need to call it pidSchemes. That's more implementation specific. Why not call it useHardwareKeySchemes? Or just pass useHardwareKey: true/false to this method? In receive pid use case we pass true, by default it's false

@berendsliedrecht berendsliedrecht force-pushed the check-vct-scheme branch 3 times, most recently from 18ab52b to 49c888b Compare August 13, 2024 12:37
pidSchemes?.sdJwtVcVcts.includes(offeredCredentialConfiguration.vct)

// TODO: add mso-mdoc config from above
const shouldKeyBeHardwareBacked = shouldKeyBeHardwareBackedForSdJwtVc ?? shouldKeyBeHardwareBackedForMsoMdoc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?? will only go to the next one if the value is undefined, but shouldKeyBeHardwareBackedForSdJwtVC will always have a value. I think you mean || in this case?

@berendsliedrecht berendsliedrecht merged commit a8232fe into main Aug 14, 2024
1 check passed
@berendsliedrecht berendsliedrecht deleted the check-vct-scheme branch August 14, 2024 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants