feat: signature-based UN and possibility of adding context to UN #284
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… optional context for both MAC-based UNs and signature-based UNs
jot2re
added
enhancement
|
@micwallace can you have a look at writing the JS code for this PR? |
Coverage report for
|
St.❔ |
Category | Percentage | Covered / Total |
|---|---|---|---|
| 🟡 | Statements | 72.8% (+40.05% 🔼) |
1965/2699 |
| 🔴 | Branches | 46.25% (+30.32% 🔼) |
265/573 |
| 🟡 | Functions | 78.9% (+52.67% 🔼) |
359/455 |
| 🟡 | Lines | 73.02% (+38.22% 🔼) |
1927/2639 |
Show new covered files 🐣
St.❔ |
File | Statements | Branches | Functions | Lines |
|---|---|---|---|---|---|
| 🔴 | libs/KeyPair.ts | 56.25% | 39.68% | 62.5% | 56.5% |
| 🔴 | libs/DerUtility.ts | 55.8% | 52% | 40% | 56.72% |
| 🟢 | ... / AttestationFramework.ts |
100% | 100% | 100% | 100% |
| 🟢 | ... / AuthenticationFramework.ts |
100% | 100% | 100% | 100% |
| 🟢 | ... / InformationFramework.ts |
100% | 100% | 100% | 100% |
| 🟢 | ... / Signature.ts |
100% | 100% | 100% | 100% |
| 🟡 | Authenticator.ts | 61.73% | 29.63% | 84.62% | 61.73% |
| 🟢 | Ticket.ts | 84.51% | 50% | 77.78% | 84.51% |
| 🟡 | ... / AttestationCrypto.ts |
66.42% | 37.04% | 90.91% | 66.67% |
| 🟢 | libs/interfaces.ts | 100% | 100% | 100% | 100% |
| 🟡 | ... / FullProofOfExponent.ts |
67.86% | 53.33% | 78.57% | 67.86% |
| 🟢 | ... / ProofOfExponentASN.ts |
100% | 100% | 100% | 100% |
| 🔴 | ... / UsageProofOfExponent.ts |
58.33% | 50% | 60% | 58.33% |
| 🔴 | ... / AttestableObject.ts |
33.33% | 100% | 33.33% | 33.33% |
| 🟢 | ... / SignedDevconTicket.ts |
100% | 100% | 100% | 100% |
| 🟢 | ... / SignedIdentifierAttestation.ts |
90.7% | 71.43% | 100% | 90.7% |
| 🟡 | ... / IdentifierAttestation.ts |
76.09% | 43.75% | 86.67% | 77.78% |
| 🟡 | libs/Attestation.ts | 65.41% | 45.59% | 90.91% | 65.91% |
| 🟡 | libs/Timestamp.ts | 73.81% | 44.44% | 70% | 73.17% |
| 🔴 | ... / AttestedObject.ts |
49.44% | 0% | 50% | 49.44% |
| 🟡 | ... / AttestationRequest.ts |
77.55% | 0% | 77.78% | 77.55% |
| 🟢 | ... / AttestationRequest.ts |
100% | 100% | 100% | 100% |
| 🟡 | libs/Nonce.ts | 73.21% | 46.67% | 100% | 73.08% |
| 🔴 | ... / SignatureUtility.ts |
53.42% | 9.09% | 30% | 54.17% |
| 🔴 | ... / ValidationTools.ts |
37.5% | 0% | 33.33% | 37.5% |
| 🟡 | ... / Eip712AttestationRequest.ts |
78.21% | 46.15% | 92.86% | 79.22% |
| 🟢 | libs/Eip712Token.ts | 100% | 100% | 100% | 100% |
| 🔴 | ... / Eip712Validator.ts |
33.33% | 34.78% | 28.57% | 34.92% |
| 🟢 | ... / UseToken.ts |
100% | 100% | 100% | 100% |
| 🟢 | data/tokenData.ts | 100% | 100% | 100% | 100% |
| 🟢 | ... / UseAttestation.ts |
92.31% | 66.67% | 90.91% | 92.31% |
| 🟢 | ... / UseAttestation.ts |
100% | 100% | 100% | 100% |
| 🟡 | ... / Eip712AttestationUsage.ts |
73.79% | 29.41% | 100% | 73.79% |
| 🟡 | ... / Eip712AttestationRequestWithUsage.ts |
71.28% | 36.84% | 93.75% | 72.04% |
| 🟢 | ... / AttestationRequestWithUsage.ts |
89.13% | 0% | 100% | 89.13% |
| 🟢 | ... / AttestationRequestWithUsage.ts |
100% | 100% | 100% | 100% |
| 🟡 | libs/ERC721Token.ts | 61.11% | 0% | 50% | 61.11% |
| 🟢 | ... / NFTAttestation.ts |
100% | 100% | 100% | 100% |
| 🟢 | ... / NFTAttestation.ts |
89.19% | 0% | 100% | 89.19% |
| 🟢 | ... / SignedNFTAttestation.ts |
88.89% | 76.19% | 100% | 88.89% |
| 🟢 | ... / SignedNFTAttestation.ts |
100% | 100% | 100% | 100% |
| 🟢 | ... / CompressedMsgSignature.ts |
80.95% | 100% | 55.56% | 80.95% |
| 🟢 | ... / AbstractSignature.ts |
88.89% | 100% | 83.33% | 88.89% |
| 🟢 | ... / PersonalSignature.ts |
100% | 100% | 100% | 100% |
| 🟢 | ... / RawSignature.ts |
83.33% | 100% | 66.67% | 83.33% |
| 🟢 | libs/Issuer.ts | 80% | 0% | 100% | 80% |
| 🟢 | ... / PublicIdentifierProof.ts |
85.19% | 0% | 100% | 85.19% |
| 🟢 | ... / IUnpredictableNumberTool.ts |
100% | 100% | 100% | 100% |
| 🟢 | libs/UNMac.ts | 100% | 100% | 100% | 100% |
| 🟢 | libs/UNSignature.ts | 100% | 100% | 100% | 100% |
Test suite run success
48 tests passing in 2 suites.
Report generated by 🧪jest coverage report action from 3a443b3
…ar (#283) * test: implemented an object dumber and updated java tests to write object dumps, along with handling linter feedback on the touched files * ci: 🎡 trigger tn CI after prerelease Co-authored-by: Tore Kasper Frederiksen <[email protected]> Co-authored-by: Feng Yu <[email protected]>
… optional context for both MAC-based UNs and signature-based UNs
|
Note that I have changed the size of the UN from 64 bits to 128 bits due to security issues. |
added 9 commits
August 28, 2022 18:24
…tween TS and Java
…ed UNs and added tests
…th Java implementation
|
@micwallace can you have a look at the PR whenever you have time? In particular the JS code. Feel free to make improvements to the JS since I am not experienced in that language |
micwallace
approved these changes
Sep 14, 2022
There was a problem hiding this comment.
@jot2re This looks good to me Tore, please feel free to merge and I'll get started with adding UN signature method to SafeConnect as part of the security review. We should also think about updating crypto-verify as well.
On customizing the signing message, I'll add this into SafeConnect for now, as a pathfinder for finding the right way to do it in this library. I forgot that it's actually in crypto-verify at the moment rather than this library. When your Java verification server is ready we will need to have a way to customize this at the application level. Maybe passing a simple template string as a parameter is the way to go.
|
@micwallace yes, I agree, that a template string is probably the right way to go. I will make an issue for this |
jot2re
added a commit
that referenced
this pull request
Nov 14, 2022
* feat: added sourcify-verify * chore: remove stub empty line * fix: 🐛 issue#276 fix. Added eip712 domain validations ✅ Closes: #276 * docs: ✏️ removed commented code, added comment to explain undef * fix: 🐛 add getters/setters for EIP712 params * ci: 🎡 trigger tn CI after prerelease * test: adding dumping of generated material for tests in attestation.jar (#283) * test: implemented an object dumber and updated java tests to write object dumps, along with handling linter feedback on the touched files * ci: 🎡 trigger tn CI after prerelease Co-authored-by: Tore Kasper Frederiksen <[email protected]> Co-authored-by: Feng Yu <[email protected]> * bug: added domain validation to AttestationUsage and AttestationRequestWUsage (#285) * ci: 🎡 trigger tn CI after prerelease * test: adding dumping of generated material for tests in attestation.jar (#283) * test: implemented an object dumber and updated java tests to write object dumps, along with handling linter feedback on the touched files * ci: 🎡 trigger tn CI after prerelease Co-authored-by: Tore Kasper Frederiksen <[email protected]> Co-authored-by: Feng Yu <[email protected]> * bug: added domain validation to AttestationUsage and AttestationRequestWUsage * fix: 🐛 issue#276 fix. Added eip712 domain validations ✅ Closes: #276 * docs: ✏️ removed commented code, added comment to explain undef * fix: 🐛 add getters/setters for EIP712 params Co-authored-by: Feng Yu <[email protected]> Co-authored-by: Tore Kasper Frederiksen <[email protected]> Co-authored-by: Oleh Hryb <[email protected]> * fix: added domain validation to AttestationRequestWithUsage and AttestationUsage * fix: 🐛 asn_decoding fix(patch), attestation timestamp check (#273) Added patch to fix ASN1_schema bug for Integers, added Attestation validate timestamp * fix: fixed integration tests with attested object (#290) Co-authored-by: Tore Kasper Frederiksen <[email protected]> * fix: updated peculiar asn1 to latest version which contains the fix that is otherwise included in the patch (#294) Co-authored-by: Tore Kasper Frederiksen <[email protected]> * feat: signature-based UN and possibility of adding context to UN (#284) * feat: wrote a UN based on signature and refactored things to allow an optional context for both MAC-based UNs and signature-based UNs * test: adding dumping of generated material for tests in attestation.jar (#283) * test: implemented an object dumber and updated java tests to write object dumps, along with handling linter feedback on the touched files * ci: 🎡 trigger tn CI after prerelease Co-authored-by: Tore Kasper Frederiksen <[email protected]> Co-authored-by: Feng Yu <[email protected]> * feat: wrote a UN based on signature and refactored things to allow an optional context for both MAC-based UNs and signature-based UNs * docs: added comments * fix: made some fixes and tried to get a consistent version working between TS and Java * fix: updated java code to be able to integration test JS generated UNs * fix: updated JS code to be able to integration test with java generated UNs and added tests * Removed unnneeded test * test: updated tests and ensured consistent and deterministic code when needed * test: updated and expanded integration tests to ensure consistency with Java implementation * fix: fixed issues related to deterministic keys * test: fixed missing references in js test * fix: fixed directory typo Co-authored-by: Feng Yu <[email protected]> * feat: ✨ safeconnect (#279) * docs(core): adding asn1 draft for nftownership attestation and ethereum linking attestation * docs(core): adding asn1 draft for nftownership attestation and ethereum linking attestation in asd format as well * docs(core): fixed typos in old asd format * docs(core): updates description to reuse old type for erc721 token * docs(core): updated asn format description after discussion with Miccy * docs(core): fixed bug in old asn1 notation * docs(core): updated asd format too * refactor:core refactored representation of erc721 tokens to fit with what we also need * feat(core): added first draft of signed NFT Ownership Attestation * test(core): added sunshine tests to signed NFT Ownership Attestation * Draft JS schemas. * Bump version - JS patch release 0.3.8 * fix(core): updated format to match the new format from Miccy * Update JS schemas for safe connect. * feat(core) added draft of ethereum key link attestation along with refactoring * fix(core) added missing file * feat(core) added draft of ethereum address attestation * refactor(core) removed redundant decoder code * test(core) added test to ownership attestations * test(core) added test to key linking attestation * Fixes for node.js support - Fix window debug ENV check. - Fix uint8array to base64 encoding. * refactor: refactoring and improved testing * test: added more testing along with file writing and reading of test data * fix: changed to assume the issuer uses Ethereum keys and refactored OID identification * fix: updated failing tests * Migrate safe connect attestation and verification code WIP. * Migrate safe connect attestation and verification code WIP. * Migrate safe connect attestation and verification code WIP. * Fix parsing * Add validity checking & optional parameter validFrom. * feat: move smart contract * chore: Refactor test attestations to be generated using javascript lib. * chore: Add javascript lib tests * feature: NFT Attestation solidity library * chore: Add test mint contract & fix attestation validity issues with tests. * fix: added missing tag to java version * fix: added fixes to get the MVP integration working * test: added integration test for JS safeconnect version * feature: JS context & various other enhancements - Add optional context field to Javascript attestation creation. - Move signing function to AbstractLinkedAttestation. - Add multi-token support to JS NFT attestation creation. - Implement test to validate Java attestation in Javascript. - Add method to get linked attestation data from EthereumKeyLinkingAttestation. * chore: Bump JS pre-release version to 0.3.9-sc.3 * test: added more safeconnect integration tests * test: added integration test of JS with Java * test: added console printing of right value * fix: missing NFT_ADDRESS variable in test * test: enable JS -> Java test * feat: made chainID mandatory on ERC721 tokens * fix: make github CI/CD pass * fix: added schema for address linking * refactor: improved variable naming * feat: added support for array of tokens in java version * feature: multi-token ID support for JS and solidity library. * refactor: changed format of Erc721 token to have IDs at the end and always store things as an array * test: updated java test * chore: bump version * fix: silly bug :-( * fix: context field type * test: added test for context * test: added test for context * fix: issuer keys being overwritten when provided in base64/PEM format feat: return attested object in validateUseTicket function. * fix: remove temporary patch for asn1-schema integer parsing issue ref: ae8c45b updated peculiar asn1 to latest version which contains the fix that is otherwise included in the patch (#294). Co-authored-by: Michael Wallace <[email protected]> Co-authored-by: snowwhitedev <[email protected]> * update contracts to use standard ERC721 and in-line library to reduce gas cost. Also update tests in line with these changes (#292) * Various fixes & enhancements for Devcon use cases (#297) * feature: validateUseTicket - allow validation to skip ethAddress check. Allow skipping of ethereum address by explicitly passing null as userEthKey value. * Bump version for devcon release * Bump version for devcon release * feature: add getDevconId function to Ticket class * build: updated dependencies * build: updated dependencies * build: updated version * build: updated version * Devcon (#299) * feature: validateUseTicket - allow validation to skip ethAddress check. Allow skipping of ethereum address by explicitly passing null as userEthKey value. * Bump version for devcon release * Bump version for devcon release * feature: add getDevconId function to Ticket class * feature: allow multiple keys for validation of a single conference ID * fix/disabled blockchain enabled tests (#303) * build: upgraded dependencies Co-authored-by: snowwhitedev <[email protected]> Co-authored-by: oleggrib <[email protected]> Co-authored-by: Feng Yu <[email protected]> Co-authored-by: Michael Wallace <[email protected]> Co-authored-by: James Brown <[email protected]>
jot2re
added a commit
that referenced
this pull request
Nov 17, 2022
* feat: added sourcify-verify * chore: remove stub empty line * fix: 🐛 issue#276 fix. Added eip712 domain validations ✅ Closes: #276 * docs: ✏️ removed commented code, added comment to explain undef * fix: 🐛 add getters/setters for EIP712 params * ci: 🎡 trigger tn CI after prerelease * test: adding dumping of generated material for tests in attestation.jar (#283) * test: implemented an object dumber and updated java tests to write object dumps, along with handling linter feedback on the touched files * ci: 🎡 trigger tn CI after prerelease Co-authored-by: Tore Kasper Frederiksen <[email protected]> Co-authored-by: Feng Yu <[email protected]> * bug: added domain validation to AttestationUsage and AttestationRequestWUsage (#285) * ci: 🎡 trigger tn CI after prerelease * test: adding dumping of generated material for tests in attestation.jar (#283) * test: implemented an object dumber and updated java tests to write object dumps, along with handling linter feedback on the touched files * ci: 🎡 trigger tn CI after prerelease Co-authored-by: Tore Kasper Frederiksen <[email protected]> Co-authored-by: Feng Yu <[email protected]> * bug: added domain validation to AttestationUsage and AttestationRequestWUsage * fix: 🐛 issue#276 fix. Added eip712 domain validations ✅ Closes: #276 * docs: ✏️ removed commented code, added comment to explain undef * fix: 🐛 add getters/setters for EIP712 params Co-authored-by: Feng Yu <[email protected]> Co-authored-by: Tore Kasper Frederiksen <[email protected]> Co-authored-by: Oleh Hryb <[email protected]> * fix: added domain validation to AttestationRequestWithUsage and AttestationUsage * fix: 🐛 asn_decoding fix(patch), attestation timestamp check (#273) Added patch to fix ASN1_schema bug for Integers, added Attestation validate timestamp * fix: fixed integration tests with attested object (#290) Co-authored-by: Tore Kasper Frederiksen <[email protected]> * fix: updated peculiar asn1 to latest version which contains the fix that is otherwise included in the patch (#294) Co-authored-by: Tore Kasper Frederiksen <[email protected]> * feat: signature-based UN and possibility of adding context to UN (#284) * feat: wrote a UN based on signature and refactored things to allow an optional context for both MAC-based UNs and signature-based UNs * test: adding dumping of generated material for tests in attestation.jar (#283) * test: implemented an object dumber and updated java tests to write object dumps, along with handling linter feedback on the touched files * ci: 🎡 trigger tn CI after prerelease Co-authored-by: Tore Kasper Frederiksen <[email protected]> Co-authored-by: Feng Yu <[email protected]> * feat: wrote a UN based on signature and refactored things to allow an optional context for both MAC-based UNs and signature-based UNs * docs: added comments * fix: made some fixes and tried to get a consistent version working between TS and Java * fix: updated java code to be able to integration test JS generated UNs * fix: updated JS code to be able to integration test with java generated UNs and added tests * Removed unnneeded test * test: updated tests and ensured consistent and deterministic code when needed * test: updated and expanded integration tests to ensure consistency with Java implementation * fix: fixed issues related to deterministic keys * test: fixed missing references in js test * fix: fixed directory typo Co-authored-by: Feng Yu <[email protected]> * feat: ✨ safeconnect (#279) * docs(core): adding asn1 draft for nftownership attestation and ethereum linking attestation * docs(core): adding asn1 draft for nftownership attestation and ethereum linking attestation in asd format as well * docs(core): fixed typos in old asd format * docs(core): updates description to reuse old type for erc721 token * docs(core): updated asn format description after discussion with Miccy * docs(core): fixed bug in old asn1 notation * docs(core): updated asd format too * refactor:core refactored representation of erc721 tokens to fit with what we also need * feat(core): added first draft of signed NFT Ownership Attestation * test(core): added sunshine tests to signed NFT Ownership Attestation * Draft JS schemas. * Bump version - JS patch release 0.3.8 * fix(core): updated format to match the new format from Miccy * Update JS schemas for safe connect. * feat(core) added draft of ethereum key link attestation along with refactoring * fix(core) added missing file * feat(core) added draft of ethereum address attestation * refactor(core) removed redundant decoder code * test(core) added test to ownership attestations * test(core) added test to key linking attestation * Fixes for node.js support - Fix window debug ENV check. - Fix uint8array to base64 encoding. * refactor: refactoring and improved testing * test: added more testing along with file writing and reading of test data * fix: changed to assume the issuer uses Ethereum keys and refactored OID identification * fix: updated failing tests * Migrate safe connect attestation and verification code WIP. * Migrate safe connect attestation and verification code WIP. * Migrate safe connect attestation and verification code WIP. * Fix parsing * Add validity checking & optional parameter validFrom. * feat: move smart contract * chore: Refactor test attestations to be generated using javascript lib. * chore: Add javascript lib tests * feature: NFT Attestation solidity library * chore: Add test mint contract & fix attestation validity issues with tests. * fix: added missing tag to java version * fix: added fixes to get the MVP integration working * test: added integration test for JS safeconnect version * feature: JS context & various other enhancements - Add optional context field to Javascript attestation creation. - Move signing function to AbstractLinkedAttestation. - Add multi-token support to JS NFT attestation creation. - Implement test to validate Java attestation in Javascript. - Add method to get linked attestation data from EthereumKeyLinkingAttestation. * chore: Bump JS pre-release version to 0.3.9-sc.3 * test: added more safeconnect integration tests * test: added integration test of JS with Java * test: added console printing of right value * fix: missing NFT_ADDRESS variable in test * test: enable JS -> Java test * feat: made chainID mandatory on ERC721 tokens * fix: make github CI/CD pass * fix: added schema for address linking * refactor: improved variable naming * feat: added support for array of tokens in java version * feature: multi-token ID support for JS and solidity library. * refactor: changed format of Erc721 token to have IDs at the end and always store things as an array * test: updated java test * chore: bump version * fix: silly bug :-( * fix: context field type * test: added test for context * test: added test for context * fix: issuer keys being overwritten when provided in base64/PEM format feat: return attested object in validateUseTicket function. * fix: remove temporary patch for asn1-schema integer parsing issue ref: ae8c45b updated peculiar asn1 to latest version which contains the fix that is otherwise included in the patch (#294). Co-authored-by: Michael Wallace <[email protected]> Co-authored-by: snowwhitedev <[email protected]> * update contracts to use standard ERC721 and in-line library to reduce gas cost. Also update tests in line with these changes (#292) * Various fixes & enhancements for Devcon use cases (#297) * feature: validateUseTicket - allow validation to skip ethAddress check. Allow skipping of ethereum address by explicitly passing null as userEthKey value. * Bump version for devcon release * Bump version for devcon release * feature: add getDevconId function to Ticket class * build: updated dependencies * build: updated dependencies * build: updated version * build: updated version * Devcon (#299) * feature: validateUseTicket - allow validation to skip ethAddress check. Allow skipping of ethereum address by explicitly passing null as userEthKey value. * Bump version for devcon release * Bump version for devcon release * feature: add getDevconId function to Ticket class * feature: allow multiple keys for validation of a single conference ID * fix/disabled blockchain enabled tests (#303) * build: upgraded dependencies * fix: 🐛 hotfix - updated @types/node * Fix/breaking change erc721 (#304) * fix: changes to use a legacy format for ERC721 for legacy alchemy NFT tokens * fix: added missing file * fix: fix javascript to also use legacy format in the legacy setting * build: updated versions * fix: reversal experiment to appease github actions * fix: second attempt * build: updated non-breaking dependencies * fix: changed access of ERC721 * fix: 🐛 hotfix - updated @types/node Co-authored-by: Oleh Hryb <[email protected]> Co-authored-by: snowwhitedev <[email protected]> Co-authored-by: oleggrib <[email protected]> Co-authored-by: Feng Yu <[email protected]> Co-authored-by: Michael Wallace <[email protected]> Co-authored-by: James Brown <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds another type of UN, where is signature-based and hence can be validated on-chain.
The PR also adds the option of adding a context to a UN as well.
This closes issue #275 and issue #259