fix: set check ethr signer for node clients #618

Harasz · 2022-07-22T10:02:54Z

Description

Add missing set for ethr signer for node clients

Contributor checklist

Breaking changes - check for any existing interfaces changes that are not backward compatible, removed method etc.
Documentation - document your code, add comments for method, remember to check if auto generated docs were updated.
Tests - add new or updated existed test for changes you made.
Migration guide - add migration guide for every breaking change.
Configuration correctness - check that any configuration changes are correct ex. default URLs, chain ids, smart contract verification on Volta explorer or EWC explorer.

JGiter · 2022-07-22T10:53:56Z

src/modules/signer/signer.service.ts

+   * Set `_isEthSigner` value based on a signed message.
+   * Generates a test message and signs it.
+   */
+  private async _setIsEthrSigner() {


Why not use _calculatePubKeyAndIdentityToken?

We can, but _calculatePubKeyAndIdentityToken is making a call to RPC asking about block number, so for performance reason i decide to chose this approach

jrhender · 2022-07-22T11:17:38Z

src/modules/signer/signer.service.ts

+    const header = {
+      alg: 'ES256',
+      typ: 'JWT',
+    };
+    const encodedHeader = base64url(JSON.stringify(header));
+    const address = this._address;
+    const payload = {
+      address,
+    };
+
+    const encodedPayload = base64url(JSON.stringify(payload));
+    const token = `0x${Buffer.from(
+      `${encodedHeader}.${encodedPayload}`
+    ).toString('hex')}`;


I don't think we this JWT stuff. I think we just need to sign a message and see if we can recover the address. Is there an advantage to the JWT bit?

Yeah, you are right. simple message is enough

jrhender · 2022-07-22T14:19:41Z

The code looks good, but how hard would it be to add tests for this case?
Sorry, I should have asked this in my initial feedback.

JGiter · 2022-07-22T14:31:11Z

The code looks good, but how hard would it be to add tests for this case?

I think the simplest way to test is to remove code which currently sets isEthrSigner, like here

iam-client-lib/e2e/claims.service.e2e.ts

Line 208 in e61ff2b

await signerService.publicKeyAndIdentityToken();

Harasz · 2022-07-25T09:28:11Z

@jrhender tests added

jrhender

Looks good, thanks @Harasz

## [6.0.0-alpha.42](v6.0.0-alpha.41...v6.0.0-alpha.42) (2022-07-25) ### Bug Fixes * set check ethr signer for node clients ([#618](#618)) ([c198dc7](c198dc7))

ewf-devops · 2022-07-25T10:00:43Z

🎉 This PR is included in version 6.0.0-alpha.42 🎉

The release is available on:

Your semantic-release bot 📦🚀

## [6.0.0](v5.0.0...v6.0.0) (2022-08-15) ### ⚠ BREAKING CHANGES * **infura:** Requiring object instead of string for IPFS config * fix(infura): allow DID reg to take in Infura config * fix(infura): create helper method for ipfs config for tests * fix(infura): add return type * fix(infura): use IFPS daemon for tests * fix(infura): use ipfs config for initUser methods * fix(infura): replace config with ipfs daemon for initUser methods * `VerifiableCredentialsService` is initialized earlier. ### Features * add claim revocation ([2ab1d93](2ab1d93)) * add createdAt property to Claim interface ([c71344d](c71344d)) * add credential revocation ([1f8f028](1f8f028)) * add documentation ([543494f](543494f)) * add documentation comments in dist ([cc34e96](cc34e96)) * add logs for axios errors ([2fa44c6](2fa44c6)) * add method to get claims by revoker to cache client interface ([e24b506](e24b506)) * adding more details to error log line when http request fails ([#617](#617)) ([e99ceda](e99ceda)) * **addStatusToClaim:** add cred status to claim object ([f1f6328](f1f6328)) * apply prettier formatting ([abc5154](abc5154)) * **assets:** fix assets service interfaces ([c9c6a87](c9c6a87)) * **assets:** get asset owner ([1eb4832](1eb4832)) * authorize issuer ([7e8b864](7e8b864)) * claim expieration ([#567](#567)) ([757de5a](757de5a)) * **claim:** issue VC when approving role request ([a3eefbc](a3eefbc)) * **claims:** add status to vc ([28c556f](28c556f)) * **claims:** verify credential status ([b5da79f](b5da79f)) * **claims:** verify role vc ([b318dbd](b318dbd)) * continue credentials exchange ([fd07ba0](fd07ba0)) * **docs:** quick start ([cd85c17](cd85c17)) * **domain-reader:** refactor domainreader initialisation ([9f7ea6e](9f7ea6e)) * **domains:** add MulticallTx type ([3be1e6f](3be1e6f)) * ens scripts ([d7f0df3](d7f0df3)) * **exp:** add explainer for expiration ([5441a4f](5441a4f)) * **exp:** apply prettier formatting ([7261112](7261112)) * **exp:** fix issuerVerified as boolean ([d82eeeb](d82eeeb)) * export logger ([819e67c](819e67c)) * **exp:** remove method for expiration check; decrease test await ([9ae092c](9ae092c)) * handle unexpected did service endpoint ([#610](#610)) ([a360e93](a360e93)) * **infura:** allow DID reg to take in Infura config ([#638](#638)) ([ad2ed1a](ad2ed1a)) * initiate credential exchange ([099eb64](099eb64)) * remove verifiable credentials storage ([9501481](9501481)) * **revocation:** add credentialStatus to EIP191JWT ([52fec66](52fec66)) * **RevokeClaim:** add endpoint to fetch claims by revoker (current user) ([80cf934](80cf934)) * **revokeClaim:** make did param required ([b6648d6](b6648d6)) * types in claims service (EthersProviderIssuerResolver) ([11ac865](11ac865)) * update claim revocation registry address ([effe1d8](effe1d8)) * update types and interfaces from ew-credentials ([0ed9855](0ed9855)) * **vc:** add verifiable credential expiration date ([e76c66b](e76c66b)) * verifiable credential poc ([a7f8df7](a7f8df7)) * verifiable presentation poc ([a4732e3](a4732e3)) * **verifiable-credentials:** add credential status eip712 type ([d9db01f](d9db01f)) * **verifiable-credentials:** get credentials by definition ([5f2e7a8](5f2e7a8)) * **verifyExp:** add documentation; address PR comments ([e7a50a5](e7a50a5)) * **verifyExp:** add exp verification method and tests for EIP and VP ([7099c09](7099c09)) * **verifyExp:** remove comment ([1d96490](1d96490)) * **verifyVc:** add jsonwebtoken and update package-lock ([81ff25d](81ff25d)) * **verifyVc:** add unit tests ([c4390b1](c4390b1)) * **verifyVC:** add verifyVC method to VC Base Service ([3187e5e](3187e5e)) * **verifyVC:** make proof private; add error and return types ([0da3229](0da3229)) * **verifyVc:** remove comments and update test case descriptions ([04f9c9f](04f9c9f)) * **verifyVc:** resolve conflicts and add formatting ([b24f328](b24f328)) * **verifyVC:** update ew-credentials ([877fad6](877fad6)) * **verifyVc:** update ew-credentials version and use to resolve credentials ([cd77eb0](cd77eb0)) * **verifyVc:** update method and params ([9800557](9800557)) * **verifyVc:** update method documentation ([86e47e1](86e47e1)) * **verifyVc:** update resolve method and add method for offchain verify ([800c903](800c903)) * **verifyVc:** update unit tests ([9d5e7ba](9d5e7ba)) * **verifyVc:** update unit tests ([eb0c7e0](eb0c7e0)) * **verifyVc:** update verify method name ([e5733c6](e5733c6)) * **verifyVc:** update verifyOffChainClaim method to accept Claim interface ([3034101](3034101)) ### Bug Fixes * add checking if service endpoint is a CID ([a0b5d1a](a0b5d1a)) * add vp request to return of initiate exchange ([ddecbaf](ddecbaf)) * **addVcVerifier:** add VerifyVCIssuer method ([6052d41](6052d41)) * **addVcVerifier:** return instance of domain reader ([ce037f4](ce037f4)) * **addVcVerifier:** return type for Domain Reader ([84ba78f](84ba78f)) * **addVcVerifier:** update package-lock version ([9eb00ac](9eb00ac)) * avoid refreshing undefined token ([86bed3c](86bed3c)) * cache client authentication ([619845d](619845d)) * cache-client token refresh ([05e9f1a](05e9f1a)) * **claim:** fix claim service issuance interfaces ([4c32a26](4c32a26)) * **claim:** fix public claim publishing ([d77df34](d77df34)) * **claims:** dont reregister onchain claim ([ebd46c0](ebd46c0)) * **did-registry:** optional encoding and algo of public key ([26a239f](26a239f)) * **docs:** architecture ([f7daa96](f7daa96)) * **docs:** guides ([627bc88](627bc88)) * **domain:** fix domain service interfaces ([50a014e](50a014e)) * don't remove deploy badge ([c216ed8](c216ed8)) * exchange credential tests grouping ([2b1e674](2b1e674)) * **exp:** do not use dflt validity period to calculate timestamp if n… ([#623](#623)) ([9ce524d](9ce524d)) * filter self-signed credential from presDef before sending to PEX ([6d43aeb](6d43aeb)) * fix backward compatibility for did registry interface ([a24a91c](a24a91c)) * fix delegate token payload ([56bfb1d](56bfb1d)) * on-chain claim revocation ([df9eeea](df9eeea)) * retry failed requests on token refresh ([434e262](434e262)) * set check ethr signer for node clients ([#618](#618)) ([c198dc7](c198dc7)) * set index page for readthedocs ([f6b4a49](f6b4a49)) * throw an error when a did document update failed ([e7c1ae9](e7c1ae9)) * **verifiable-credentials:** check errors in exchange initiating ([1bc8626](1bc8626)) * **verifiable-credentials:** check errors on initiating exchange ([bce85a8](bce85a8)) * **verifiable-credentials:** filter invalid issuer fields ([8c8b0c3](8c8b0c3)) ### Refactoring * apply naming convention ([cc41154](cc41154)) * cleanup credential exchange code ([#602](#602)) ([56444c6](56444c6)) * rename issuer credential status ([6c55143](6c55143)) ### Documentation * add changes in index.md ([0d5626a](0d5626a)) * add new index.md ([5da81dc](5da81dc)) * **asset:** improve asset service documentation ([eada9b8](eada9b8)) * browser session management ([762a42e](762a42e)) * **claim.md:** fix more links ([92b7f14](92b7f14)) * **claim:** improve claim service documentation ([fa89229](fa89229)) * **claims:** getClaimId ([affaef7](affaef7)) * copy README to docs index.md and remove QUICKSTART file (not used anywhere) ([3ac8fbf](3ac8fbf)) * **did-registry:** improve DID registry service documentation ([b0bbf6f](b0bbf6f)) * document iam initialization ([1552965](1552965)) * **domains:** improve domain service documentation ([861fbfd](861fbfd)) * fix api links in guides files ([6934d4a](6934d4a)) * improve read the docs ([#613](#613)) ([ccc2358](ccc2358)) * **index.md:** remove out of date sections ([5874ed4](5874ed4)) * install in browser application ([488106f](488106f)) * **messaging:** improve messaging service documentation ([509e201](509e201)) * organize api by modules ([aae1c23](aae1c23)) * **README.md:** update active maintainers ([1cb98c2](1cb98c2)) * **README:** fix link to read the docs ([8858048](8858048)) * **README:** fix names of "set" methods ([37126ae](37126ae)) * remove architecture intro section ([f8da541](f8da541)) * remove quickstart from mkdocs structure ([b9919e1](b9919e1)) * remove README from api ([78bc3a0](78bc3a0)) * remove trailing spaces ([3ffb6af](3ffb6af)) * **signer:** improve signer service documentation ([7a8048d](7a8048d)) * update examples role objects ([99066bf](99066bf)) * update path to logo to be relative ([4fc7413](4fc7413)) * update README template and add quick start documentation ([25b85c9](25b85c9)) * update verifiable credentials documentation ([a1d5a5e](a1d5a5e)) * **vc:** improve verifiable credential service documentation ([f7191f1](f7191f1))

ewf-devops · 2022-08-15T09:25:37Z

🎉 This PR is included in version 6.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Harasz requested review from jrhender and JGiter July 22, 2022 10:02

JGiter reviewed Jul 22, 2022

View reviewed changes

JGiter approved these changes Jul 22, 2022

View reviewed changes

jrhender requested changes Jul 22, 2022

View reviewed changes

Harasz force-pushed the task/ICL-256-set-ethr-signer-node branch from 1a962f5 to ca8c0c5 Compare July 22, 2022 12:26

Harasz requested a review from jrhender July 22, 2022 12:27

fix: set check ethr signer for node clients

25d452a

Harasz force-pushed the task/ICL-256-set-ethr-signer-node branch from ca8c0c5 to 25d452a Compare July 22, 2022 12:58

test: add ethr signer tests

6ba09a9

Harasz force-pushed the task/ICL-256-set-ethr-signer-node branch from d804c35 to 6ba09a9 Compare July 25, 2022 09:15

jrhender approved these changes Jul 25, 2022

View reviewed changes

Harasz merged commit c198dc7 into develop Jul 25, 2022

Harasz deleted the task/ICL-256-set-ethr-signer-node branch July 25, 2022 09:38

ewf-devops pushed a commit that referenced this pull request Jul 25, 2022

chore(release): 6.0.0-alpha.42 [skip ci]

0f532bb

## [6.0.0-alpha.42](v6.0.0-alpha.41...v6.0.0-alpha.42) (2022-07-25) ### Bug Fixes * set check ethr signer for node clients ([#618](#618)) ([c198dc7](c198dc7))

ewf-devops added the released on @canary label Jul 25, 2022

ewf-devops added the released on @latest label Aug 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: set check ethr signer for node clients #618

fix: set check ethr signer for node clients #618

Harasz commented Jul 22, 2022

JGiter Jul 22, 2022

Harasz Jul 22, 2022

jrhender Jul 22, 2022

Harasz Jul 22, 2022

jrhender commented Jul 22, 2022

JGiter commented Jul 22, 2022

Harasz commented Jul 25, 2022

jrhender left a comment

ewf-devops commented Jul 25, 2022

ewf-devops commented Aug 15, 2022

fix: set check ethr signer for node clients #618

fix: set check ethr signer for node clients #618

Conversation

Harasz commented Jul 22, 2022

Description

Contributor checklist

JGiter Jul 22, 2022

Choose a reason for hiding this comment

Harasz Jul 22, 2022

Choose a reason for hiding this comment

jrhender Jul 22, 2022

Choose a reason for hiding this comment

Harasz Jul 22, 2022

Choose a reason for hiding this comment

jrhender commented Jul 22, 2022

JGiter commented Jul 22, 2022

Harasz commented Jul 25, 2022

jrhender left a comment

Choose a reason for hiding this comment

ewf-devops commented Jul 25, 2022

ewf-devops commented Aug 15, 2022