feat(askar): improved hardware key support #1968
Conversation
Signed-off-by: Berend Sliedrecht <[email protected]>
|
| keyId: kid, | ||
| }) | ||
|
|
||
| return new Key(publicKeyBytes, keyType, keyId) |
There was a problem hiding this comment.
Should be kid here
| @@ -44,6 +45,7 @@ | |||
| "typescript": "~5.5.2" | |||
| }, | |||
| "peerDependencies": { | |||
| "@hyperledger/aries-askar-shared": "^0.2.3" | |||
| "@hyperledger/aries-askar-shared": "^0.2.3", | |||
| "@animo-id/expo-secure-environment": "^0.0.1-alpha.0" | |||
There was a problem hiding this comment.
Can we add it as optional peer dependency? https://pnpm.io/next/package_json#peerdependenciesmetaoptional
| const secureEnvironment = require('@animo-id/expo-secure-environment') as { | ||
| sign: typeof sign | ||
| generateKeypair: typeof generateKeypair | ||
| getPublicBytesForKeyId: typeof getPublicBytesForKeyId | ||
| } |
There was a problem hiding this comment.
the type is not really needed, as you never import from the .native.ts file. So it doesn't really matter to add the typing here (And it'll probably get out of date). If you do want to have the typying, i'd recommend to use import syntax:
| const secureEnvironment = require('@animo-id/expo-secure-environment') as { | |
| sign: typeof sign | |
| generateKeypair: typeof generateKeypair | |
| getPublicBytesForKeyId: typeof getPublicBytesForKeyId | |
| } | |
| const secureEnvironment = require('@animo-id/expo-secure-environment') as typeof import('@animo-id/expo-secure-environment') |
There was a problem hiding this comment.
However this still may give issues in envs where the package is not imported and then TS will complain that it can't find the types. So we should either always install the dep and only import it in native. Or we should not always install the dep, but then we should also not import the types form the package at all
| generateKeypair: (id: string) => void | ||
| } { | ||
| throw new Error( | ||
| 'expo-secure-environment cannot be imported in Node.js. Currently, there is no hardware key support for node.js' |
There was a problem hiding this comment.
| 'expo-secure-environment cannot be imported in Node.js. Currently, there is no hardware key support for node.js' | |
| '@animo.id/expo-secure-environment cannot be imported in Node.js. Currently, there is no hardware key support for node.js' |
| private async doesSecureEnvironmentKeyExist(keyId: string): Promise<boolean> { | ||
| try { | ||
| const entryObject = await this.withSession((session) => | ||
| session.fetch({ category: 'SecureEnvironmentRecord', name: keyId }) |
There was a problem hiding this comment.
| session.fetch({ category: 'SecureEnvironmentRecord', name: keyId }) | |
| session.fetch({ category: 'SecureEnvironmentKeyRecord', name: keyId }) |
| @@ -509,4 +557,28 @@ export abstract class AskarBaseWallet implements Wallet { | |||
| throw new WalletError('Error saving KeyPair record', { cause: error }) | |||
| } | |||
| } | |||
|
|
|||
| private async storeHardwareKeyById(options: { | |||
There was a problem hiding this comment.
incosistent naming hardwareKey vs secureEnvironmentKey
| @@ -8,10 +8,12 @@ import { getKeyTypeByMultiCodecPrefix, getMultiCodecPrefixByKeyType } from './mu | |||
| export class Key { | |||
| public readonly publicKey: Buffer | |||
| public readonly keyType: KeyType | |||
| public keyId: string | |||
There was a problem hiding this comment.
| public keyId: string | |
| /** the identifier of the key. If not provided in the constructor the base58 encoded public key will be used as the key identifier by default | |
| */ | |
| public keyId: string |
Yes, can pick that up later. |
Signed-off-by: Berend Sliedrecht <[email protected]>
berendsliedrecht
merged commit 1a941e7
into
openwallet-foundation:main
Signed-off-by: Berend Sliedrecht [email protected]