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
6 changes: 6 additions & 0 deletions .changeset/fifty-planes-fetch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@ckb-ccc/core": minor
---

feat(core): `Signer.fromSignature`

7 changes: 7 additions & 0 deletions .changeset/late-vans-juggle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@ckb-ccc/core": major
"@ckb-ccc/joy-id": minor
---

feat(joy-id): address info in identity

50 changes: 39 additions & 11 deletions packages/core/src/signer/ckb/verifyJoyId.ts
Original file line number Diff line number Diff line change
@@ -1,27 +1,55 @@
import { verifySignature } from "@joyid/ckb";
import {
CredentialKeyType,
SigningAlg,
verifyCredential,
verifySignature,
} from "@joyid/ckb";
import { BytesLike } from "../../bytes/index.js";
import { hexFrom } from "../../hex/index.js";

/**
* @public
*/
export function verifyMessageJoyId(
export async function verifyMessageJoyId(
message: string | BytesLike,
signature: string,
identity: string,
): Promise<boolean> {
const challenge =
typeof message === "string" ? message : hexFrom(message).slice(2);
const { publicKey, keyType } = JSON.parse(identity) as {
const { address, publicKey, keyType } = JSON.parse(identity) as {
address: string;
publicKey: string;
keyType: string;
keyType: CredentialKeyType;
};
const signatureObj = JSON.parse(signature) as {
alg: SigningAlg;
signature: string;
message: string;
};
Comment on lines +20 to +29
Copy link
Contributor

Choose a reason for hiding this comment

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

high

Using JSON.parse with type assertions (as) is not type-safe. If the identity or signature strings are malformed or don't match the expected structure, this will throw an unhandled exception at runtime, which could crash the application. It's recommended to wrap these parsing operations in a try...catch block and validate the parsed objects to handle potential errors gracefully.

For example:

  let identityObj: { address: string; publicKey: string; keyType: CredentialKeyType; };
  let signatureObj: { alg: SigningAlg; signature: string; message: string; };

  try {
    identityObj = JSON.parse(identity);
    signatureObj = JSON.parse(signature);
  } catch {
    return false;
  }

  const { address, publicKey, keyType } = identityObj;
  if (!address || !publicKey || !keyType || !signatureObj.alg || !signatureObj.signature) {
    return false;
  }


if (
!(await verifySignature({
challenge,
pubkey: publicKey,
keyType,
...signatureObj,
}))
) {
return false;
}

// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
return verifySignature({
challenge,
pubkey: publicKey,
keyType,
...JSON.parse(signature),
});
// I sincerely hope one day we can get rid of the centralized registry
const registry = address.startsWith("ckb")
? "https://api.joy.id/api/v1/"
: "https://api.testnet.joyid.dev/api/v1/";
Comment on lines +43 to +45
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Hardcoding URLs directly within the function makes the code harder to maintain and test. If these URLs change, you'll have to find and replace them in the code. It would be better to extract these URLs into named constants at the module level. This improves readability and centralizes configuration.

return verifyCredential(
{
pubkey: publicKey,
address,
keyType,
alg: signatureObj.alg,
},
registry,
);
}
17 changes: 17 additions & 0 deletions packages/core/src/signer/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,20 @@ export * from "./dummy/index.js";
export * from "./evm/index.js";
export * from "./nostr/index.js";
export * from "./signer/index.js";
export * from "./signerFromSignature.js";

import { BytesLike } from "../bytes/index.js";
import { Client } from "../client/index.js";
import { Signer as BaseSigner, Signature } from "./signer/index.js";
import { signerFromSignature } from "./signerFromSignature.js";

export abstract class Signer extends BaseSigner {
static fromSignature(
client: Client,
signature: Signature,
message?: string | BytesLike | null,
...addresses: (string | string[])[]
): Promise<Signer | undefined> {
return signerFromSignature(client, signature, message, ...addresses);
}
}
11 changes: 11 additions & 0 deletions packages/core/src/signer/signer/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,17 @@ export abstract class Signer {
}
}

static async fromSignature(
_client: Client,
_signature: Signature,
_message?: string | BytesLike | null,
..._addresses: (string | string[])[]
): Promise<Signer | undefined> {
throw Error(
"Signer.fromSignature should be override to avoid circular references",
);
}

/**
* Connects to the signer.
*
Expand Down
65 changes: 65 additions & 0 deletions packages/core/src/signer/signerFromSignature.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import { Address } from "../address/index.js";
import { BytesLike } from "../bytes/index.js";
import { Client } from "../client/index.js";
import { SignerBtcPublicKeyReadonly } from "./btc/index.js";
import { SignerCkbPublicKey, SignerCkbScriptReadonly } from "./ckb/index.js";
import { SignerDogeAddressReadonly } from "./doge/index.js";
import { SignerEvmAddressReadonly } from "./evm/index.js";
import { SignerNostrPublicKeyReadonly } from "./nostr/index.js";
import { Signature, Signer, SignerSignType } from "./signer/index.js";

/**
* Creates a signer from a signature.
*
* @param client - The client instance.
* @param signature - The signature to create the signer from.
* @param message - The message that was signed.
* @param addresses - The addresses to check against the signer.
* @returns The signer if the signature is valid and the addresses match, otherwise undefined.
* @throws Error if the signature sign type is unknown.
*/
export async function signerFromSignature(
client: Client,
signature: Signature,
message?: string | BytesLike | null,
...addresses: (string | string[])[]
): Promise<Signer | undefined> {
if (
message != undefined &&
!(await Signer.verifyMessage(message, signature))
) {
return;
}

const signer = await (async () => {
switch (signature.signType) {
case SignerSignType.EvmPersonal:
return new SignerEvmAddressReadonly(client, signature.identity);
case SignerSignType.BtcEcdsa:
return new SignerBtcPublicKeyReadonly(client, "", signature.identity);
case SignerSignType.JoyId: {
const { address } = JSON.parse(signature.identity) as {
address: string;
};
Comment on lines +41 to +43
Copy link
Contributor

Choose a reason for hiding this comment

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

high

Using JSON.parse with a type assertion is unsafe. If signature.identity is not valid JSON or doesn't contain an address property, this will cause a runtime error. It's important to handle this case to prevent the application from crashing.

Consider using a try...catch block and validating the parsed object:

      case SignerSignType.JoyId: {
        let address: string;
        try {
          const identity = JSON.parse(signature.identity);
          address = identity.address;
        } catch (e) {
          throw new Error(`Failed to parse JoyId identity: ${e}`);
        }
        if (!address) {
          throw new Error("Missing address in JoyId identity");
        }

        return new SignerCkbScriptReadonly(
          client,
          (await Address.fromString(address, client)).script,
        );
      }

return new SignerCkbScriptReadonly(
client,
(await Address.fromString(address, client)).script,
);
}
case SignerSignType.NostrEvent:
return new SignerNostrPublicKeyReadonly(client, signature.identity);
case SignerSignType.CkbSecp256k1:
return new SignerCkbPublicKey(client, signature.identity);
case SignerSignType.DogeEcdsa:
return new SignerDogeAddressReadonly(client, signature.identity);
case SignerSignType.Unknown:
throw new Error("Unknown signer sign type");
}
Comment on lines +35 to +57
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The switch statement is missing a default case. If a new SignerSignType enum member is added in the future, it will fall through the switch, signer will be undefined, and signer.getAddresses() on line 59 will throw a TypeError. To make this more robust, you should add a default case to handle any unexpected signType values by throwing an error.

For example:

    switch (signature.signType) {
      // ... other cases
      default:
        throw new Error(`Unsupported sign type: ${signature.signType as string}`);
    }

})();
const signerAddresses = await signer.getAddresses();
if (!addresses.flat().every((addr) => signerAddresses.includes(addr))) {
return;
}

return signer;
}
1 change: 1 addition & 0 deletions packages/joy-id/src/ckb/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ export class CkbSigner extends ccc.Signer {
async getIdentity(): Promise<string> {
const connection = await this.assertConnection();
return JSON.stringify({
address: connection.address,
keyType: connection.keyType,
publicKey: connection.publicKey.slice(2),
});
Expand Down