-
Notifications
You must be signed in to change notification settings - Fork 9
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
bumping web5 versions #149
Changes from 2 commits
7f56d82
c14d142
4d4923b
d7b4454
bb173ff
9be2f9e
ec69379
eae295f
025b95d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -50,17 +50,21 @@ | |
}, | ||
"dependencies": { | ||
"@tbdex/protocol": "workspace:*", | ||
"@web5/common": "0.2.1", | ||
"@web5/crypto": "0.2.2", | ||
"@web5/dids": "0.2.2", | ||
"query-string": "8.1.0" | ||
"@web5/common": "0.2.2", | ||
"@web5/credentials": "0.4.1", | ||
"@web5/crypto": "0.2.4", | ||
"@web5/dids": "0.2.4", | ||
"ms": "2.1.3", | ||
"query-string": "8.1.0", | ||
"typeid-js": "0.3.0" | ||
}, | ||
"devDependencies": { | ||
"@playwright/test": "1.34.3", | ||
"@types/chai": "4.3.5", | ||
"@types/eslint": "8.37.0", | ||
"@types/mocha": "10.0.1", | ||
"@types/sinon": "^17.0.2", | ||
"@types/ms": "0.7.34", | ||
"@types/sinon": "17.0.2", | ||
"@typescript-eslint/eslint-plugin": "5.59.0", | ||
"@typescript-eslint/parser": "5.59.0", | ||
"chai": "4.3.10", | ||
|
@@ -78,7 +82,7 @@ | |
"mkdirp": "3.0.1", | ||
"mocha": "10.2.0", | ||
"rimraf": "4.4.0", | ||
"sinon": "15.0.2", | ||
"sinon": "17.0.1", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good catch, ty! |
||
"typedoc": "0.25.0", | ||
"typedoc-plugin-markdown": "3.16.0", | ||
"typescript": "5.2.2" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,16 @@ | ||
import type { | ||
PrivateKeyJwk as Web5PrivateKeyJwk, | ||
CryptoAlgorithm, | ||
Web5Crypto, | ||
JwsHeaderParams, | ||
PrivateKeyJwk, | ||
PublicKeyJwk | ||
JwkParamsEcPrivate, | ||
JwkParamsOkpPrivate, | ||
JwkParamsEcPublic, | ||
JwkParamsOkpPublic | ||
} from '@web5/crypto' | ||
|
||
import { sha256 } from '@noble/hashes/sha256' | ||
import { Convert } from '@web5/common' | ||
import { EcdsaAlgorithm, EdDsaAlgorithm, Jose } from '@web5/crypto' | ||
import { EcdsaAlgorithm, EdDsaAlgorithm } from '@web5/crypto' | ||
import { deferenceDidUrl, isVerificationMethod } from './did-resolver.js' | ||
|
||
import canonicalize from 'canonicalize' | ||
|
@@ -44,14 +45,14 @@ export type VerifyOptions = { | |
*/ | ||
type SignerValue<T extends Web5Crypto.Algorithm> = { | ||
signer: CryptoAlgorithm, | ||
options: T, | ||
options?: T, | ||
alg: JwsHeader['alg'], | ||
crv: JsonWebKey['crv'] | ||
} | ||
|
||
const secp256k1Signer: SignerValue<Web5Crypto.EcdsaOptions> = { | ||
signer : new EcdsaAlgorithm(), | ||
options : { name: 'ECDSA', hash: 'SHA-256' }, | ||
options : { name: 'ECDSA' }, | ||
alg : 'ES256K', | ||
crv : 'secp256k1' | ||
} | ||
|
@@ -101,32 +102,30 @@ export class Crypto { | |
static async sign(opts: SignOptions) { | ||
const { did, payload, detached } = opts | ||
|
||
const privateKeyJwk = did.keySet.verificationMethodKeys?.[0]?.privateKeyJwk | ||
const privateKeyJwk = did.keySet.verificationMethodKeys[0].privateKeyJwk as JwkParamsEcPrivate | JwkParamsOkpPrivate | ||
|
||
const algorithmName = privateKeyJwk?.['alg'] || '' | ||
let namedCurve = Crypto.extractNamedCurve(privateKeyJwk) | ||
const algorithmName = privateKeyJwk['alg'] || '' | ||
const namedCurve = privateKeyJwk['crv'] || '' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CI is failing because you're reverting some of the changes from #141. Might need to rebase. |
||
const algorithmId = `${algorithmName}:${namedCurve}` | ||
|
||
const algorithm = this.algorithms[algorithmId] | ||
if (!algorithm) { | ||
throw new Error(`Algorithm (${algorithmId}) not supported`) | ||
throw new Error(`${algorithmId} not supported`) | ||
} | ||
|
||
let verificationMethodId = did.document.verificationMethod?.[0]?.id || '' | ||
let verificationMethodId = did.document.verificationMethod[0].id | ||
if (verificationMethodId.startsWith('#')) { | ||
verificationMethodId = `${did.did}#${verificationMethodId}` | ||
verificationMethodId = `${did.did}${verificationMethodId}` | ||
} | ||
|
||
const jwsHeader: JwsHeader = { alg: algorithm.alg, kid: verificationMethodId } | ||
const base64UrlEncodedJwsHeader = Convert.object(jwsHeader).toBase64Url() | ||
const base64urlEncodedJwsPayload = Convert.uint8Array(payload).toBase64Url() | ||
|
||
const key = await Jose.jwkToCryptoKey({ key: privateKeyJwk as Web5PrivateKeyJwk }) | ||
|
||
const toSign = `${base64UrlEncodedJwsHeader}.${base64urlEncodedJwsPayload}` | ||
const toSignBytes = Convert.string(toSign).toUint8Array() | ||
|
||
const signatureBytes = await algorithm.signer.sign({ key, data: toSignBytes, algorithm: algorithm.options }) | ||
const signatureBytes = await algorithm.signer.sign({ key: privateKeyJwk, data: toSignBytes, algorithm: algorithm.options }) | ||
const base64UrlEncodedSignature = Convert.uint8Array(signatureBytes).toBase64Url() | ||
|
||
if (detached) { | ||
|
@@ -170,13 +169,14 @@ export class Crypto { | |
throw new Error('Signature verification failed: Expected JWS header to contain alg and kid') | ||
} | ||
|
||
const verificationMethod = await deferenceDidUrl(jwsHeader.kid) | ||
const verificationMethod = await deferenceDidUrl(jwsHeader.kid as string) | ||
if (!isVerificationMethod(verificationMethod)) { // ensure that appropriate verification method was found | ||
throw new Error('Signature verification failed: Expected kid in JWS header to dereference to a DID Document Verification Method') | ||
} | ||
|
||
// will be used to verify signature | ||
const { publicKeyJwk } = verificationMethod | ||
const publicKeyJwk = verificationMethod.publicKeyJwk as JwkParamsEcPublic | JwkParamsOkpPublic | ||
|
||
if (!publicKeyJwk) { // ensure that Verification Method includes public key as a JWK. | ||
throw new Error('Signature verification failed: Expected kid in JWS header to dereference to a DID Document Verification Method with publicKeyJwk') | ||
} | ||
|
@@ -186,37 +186,19 @@ export class Crypto { | |
|
||
const signatureBytes = Convert.base64Url(base64UrlEncodedSignature).toUint8Array() | ||
|
||
const algorithmId = `${jwsHeader['alg']}:${Crypto.extractNamedCurve(publicKeyJwk)}` | ||
const algorithmId = `${jwsHeader['alg']}:${publicKeyJwk['crv']}` | ||
const { signer, options } = Crypto.algorithms[algorithmId] | ||
|
||
// TODO: remove this monkeypatch once 'ext' is no longer a required property within a jwk passed to `jwkToCryptoKey` | ||
const monkeyPatchPublicKeyJwk = { | ||
...publicKeyJwk, | ||
ext : 'true' as const, | ||
key_ops : ['verify'] | ||
} | ||
|
||
const key = await Jose.jwkToCryptoKey({ key: monkeyPatchPublicKeyJwk }) | ||
const isLegit = await signer.verify({ algorithm: options, key, data: signedDataBytes, signature: signatureBytes }) | ||
const isLegit = await signer.verify({ algorithm: options, key: publicKeyJwk, data: signedDataBytes, signature: signatureBytes }) | ||
|
||
if (!isLegit) { | ||
throw new Error('Signature verification failed: Integrity mismatch') | ||
} | ||
|
||
const [did] = jwsHeader.kid.split('#') | ||
const [did] = (jwsHeader as JwsHeaderParams).kid.split('#') | ||
return did | ||
} | ||
|
||
/** | ||
* Gets crv property from a PublicKeyJwk or PrivateKeyJwk. Returns empty string if crv is undefined. | ||
*/ | ||
static extractNamedCurve(jwk: PrivateKeyJwk | PublicKeyJwk | undefined): string { | ||
if (jwk && 'crv' in jwk) { | ||
return jwk.crv | ||
} else { | ||
return '' | ||
} | ||
} | ||
} | ||
|
||
/** | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we adding
ms
andtypeid-js
tohttp-client
in this PR? Ifms
is necessary independencies
, why is@types/ms
indevDepencencies
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so this PR is part of a 4 PR series in an effort to break down moe's 122-request-token PR which is a chonky boi.
i just ported over the client/package.json changes from the original PR, which includes ms and typeid-js since they're being used by the client during request token generation.
the PR that will use the ms and typeid-js is here
i thought we needed to include @types/xyz in devDependencies that correspond to package xyz in dependencies in order to write with type checking during development. is that incorrect?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, u right
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yee that's correct @jiyoontbd