Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/askar-native-instance.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@credo-ts/askar': minor
---

Read the askar native binding via `NativeAskar.instance` instead of the deprecated mutable `askar` export in the KMS (key management and ECDH key derivation). The deprecated binding is populated only when the platform package's side-effect import runs, so an ESM consumer that loads the KMS first snapshots it as `undefined` and key operations fail with `Cannot read properties of undefined (reading 'keyGetJwkSecret')`. `NativeAskar.instance` resolves the binding on each access, so registration order no longer matters. See #2597, #2607.

This raises the minimum required `@openwallet-foundation/askar-shared` to 0.6.0, where `NativeAskar` was introduced, so the peer dependency range is narrowed to `^0.6.0`.
2 changes: 1 addition & 1 deletion packages/askar/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,6 @@
"typescript": "catalog:"
},
"peerDependencies": {
"@openwallet-foundation/askar-shared": "^0.4.3 || ^0.5.0 || ^0.6.0"
"@openwallet-foundation/askar-shared": "^0.6.0"
}
}
12 changes: 8 additions & 4 deletions packages/askar/src/kms/AskarKeyManagementService.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { type AgentContext, JsonEncoder, Kms, TypedArrayEncoder, utils } from '@credo-ts/core'
import type { JwkProps, KeyEntryObject, Session } from '@openwallet-foundation/askar-shared'
import {
askar,
CryptoBox,
Jwk,
Key,
KeyAlgorithm,
KeyEntryList,
NativeAskar,
SignatureAlgorithm,
} from '@openwallet-foundation/askar-shared'

Expand Down Expand Up @@ -681,7 +681,7 @@ export class AskarKeyManagementService implements Kms.KeyManagementService {

private keyFromJwk(jwk: Kms.KmsJwkPrivate | Kms.KmsJwkPublic) {
const key = new Key(
askar.keyFromJwk({
NativeAskar.instance.keyFromJwk({
// TODO: the JWK class in JS Askar wrapper is too limiting
// so we use this method directly. should update it
jwk: new Uint8Array(JsonEncoder.toUint8Array(jwk)) as unknown as Jwk,
Expand Down Expand Up @@ -717,7 +717,7 @@ export class AskarKeyManagementService implements Kms.KeyManagementService {
// so we use this method directly. should update it
// We extract alg, as Askar doesn't always use the same algs
const { alg, ...jwkSecret } = JsonEncoder.fromUint8Array(
askar.keyGetJwkSecret({
NativeAskar.instance.keyGetJwkSecret({
localKeyHandle: key.handle,
})
)
Expand All @@ -733,7 +733,11 @@ export class AskarKeyManagementService implements Kms.KeyManagementService {
if (!session.handle) throw Error('Cannot fetch a key with a closed session')

// Fetch the key from the session
const handle = await askar.sessionFetchKey({ forUpdate: false, name: keyId, sessionHandle: session.handle })
const handle = await NativeAskar.instance.sessionFetchKey({
forUpdate: false,
name: keyId,
sessionHandle: session.handle,
})
if (!handle) return null

// Get the key entry
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { readFileSync } from 'node:fs'
import path from 'node:path'
import { InjectionSymbols, JsonEncoder, Kms, TypedArrayEncoder } from '@credo-ts/core'
import { AskarError, askar } from '@openwallet-foundation/askar-shared'
import { AskarError, NativeAskar } from '@openwallet-foundation/askar-shared'
import { getAgentConfig, getAgentContext } from '../../../../core/tests'
import { NodeFileSystem } from '../../../../node/src/NodeFileSystem'
import { AskarModuleConfig, AskarMultiWalletDatabaseScheme } from '../../AskarModuleConfig'
Expand All @@ -17,7 +17,7 @@ const agentContext = getAgentContext({
AskarModuleConfig,
new AskarModuleConfig({
multiWalletDatabaseScheme: AskarMultiWalletDatabaseScheme.ProfilePerWallet,
askar,
askar: NativeAskar.instance,
store: {
id: 'default',
key: 'CwNJroKHTSSj3XvE7ZAnuKiTn2C4QkFvxEqfm5rzhNrb',
Expand Down
6 changes: 3 additions & 3 deletions packages/askar/src/kms/crypto/deriveKey.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Kms, TypedArrayEncoder } from '@credo-ts/core'
import { askar, Key } from '@openwallet-foundation/askar-shared'
import { Key, NativeAskar } from '@openwallet-foundation/askar-shared'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we should try to read Askar from the module config: https://github.com/openwallet-foundation/credo-ts/blob/main/packages/askar/src/AskarModuleConfig.ts#L130

  • inject Moduleconfig on agent context
  • extract askar instance from it
  • We should probably update the askar getter in the module config, to see if Askar is passed or the NativeAskar?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, reading the injected instance from AskarModuleConfig is cleaner than the global getter,.

Two things I want to confirm before I push it:

  1. Scope. deriveKey, fetchAskarKey, and encrypt/decrypt all have agentContext, so those are straightforward. But keyFromJwk, privateJwkFromKey, and publicJwkFromKey don't receive agentContext, and they're called from ~13 spots across getPublicKey/createKey/sign/verify/encrypt/decrypt. Doing this consistently means threading the resolved instance through those helpers too (doing it only in deriveKey would leave the JWK paths on the global, which defeats the purpose). Good with that?

  2. The getter. askar is currently a required option, so config.askar always resolves and a fallback isn't strictly needed. Do you want me to make it optional and fall back to NativeAskar.instance when it isn't passed? Heads up that this decides the peer range: with the fallback we keep referencing NativeAskar, so askar-shared stays at ^0.6.0 (changeset minor); if askar stays required, nothing references NativeAskar anymore and the peer can go back to the broad range (patch).

import { jwkEncToAskarAlg } from '../../utils'

export const askarSupportedKeyAgreementAlgorithms = [
Expand Down Expand Up @@ -47,7 +47,7 @@ export function deriveEncryptionKey(options: {
: undefined

const derivedKey = new Key(
askar.keyDeriveEcdhEs({
NativeAskar.instance.keyDeriveEcdhEs({
algId: TypedArrayEncoder.fromUtf8String(
keyAgreement.algorithm === 'ECDH-ES' ? encryption.algorithm : keyAgreement.algorithm
),
Expand Down Expand Up @@ -130,7 +130,7 @@ export function deriveDecryptionKey(options: {
: undefined

const derivedKey = new Key(
askar.keyDeriveEcdhEs({
NativeAskar.instance.keyDeriveEcdhEs({
algId: TypedArrayEncoder.fromUtf8String(
keyAgreement.algorithm === 'ECDH-ES' ? decryption.algorithm : keyAgreement.algorithm
),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { AgentContext, TagsBase } from '@credo-ts/core'

import { RecordDuplicateError, RecordNotFoundError, TypedArrayEncoder } from '@credo-ts/core'
import { askar } from '@openwallet-foundation/askar-nodejs'
import { NativeAskar } from '@openwallet-foundation/askar-nodejs'

import { TestRecord } from '../../../../core/src/storage/__tests__/TestRecord'
import { getAgentConfig, getAgentContext, getAskarStoreConfig } from '../../../../core/tests/helpers'
Expand All @@ -27,7 +27,7 @@ describe('AskarStorageService', () => {
storeManager = new AskarStoreManager(
new NodeFileSystem(),
new AskarModuleConfig({
askar,
askar: NativeAskar.instance,
store: getAskarStoreConfig('AskarStorageServiceTest', {
inMemory: true,
}),
Expand Down Expand Up @@ -70,7 +70,7 @@ describe('AskarStorageService', () => {
})

const retrieveRecord = await storeManager.withSession(agentContext, (session) =>
askar.sessionFetch({
NativeAskar.instance.sessionFetch({
category: record.type,
name: record.id,
// biome-ignore lint/style/noNonNullAssertion: no explanation
Expand All @@ -96,7 +96,7 @@ describe('AskarStorageService', () => {
await storeManager.withSession(
agentContext,
async (session) =>
await askar.sessionUpdate({
await NativeAskar.instance.sessionUpdate({
category: TestRecord.type,
name: 'some-id',
// biome-ignore lint/style/noNonNullAssertion: no explanation
Expand Down
Loading