Skip to content
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

refactor: Delete legacy operation related to credentials #831

Merged

Conversation

EzequielPostan
Copy link
Contributor

@EzequielPostan EzequielPostan commented Apr 3, 2024

This PR deletes the IssueCredentialBatchOperation and RevokeCredentialsOperation. It also deletes node API, daos and repositories related to credentials. The PR removed the dependency on prism-credentials, prism-protos, prism-api and prism-vault. It added the dependencies on prism-identity and prism-crypto to be removed in follow up PRs

Overview

Screenshots

Checklists

Pre-submit checklist:

  • Self-reviewed the diff
  • New code has proper comments/documentation/tests
  • Any changes not covered by tests have been tested manually
  • The README files are updated
  • If new libraries are included, they have licenses compatible with our project
  • If there is a db migration altering existing tables, there is a proper migration test

Pre-merge checklist:

  • Commits have useful messages
  • Review clarifications made it into the code

@EzequielPostan EzequielPostan requested a review from shotexa April 3, 2024 01:23
Copy link

github-actions bot commented Apr 3, 2024

Unit Test Results

273 tests   272 ✔️  24s ⏱️
  38 suites      1 💤
  38 files        0

Results for commit 6e47a10.

♻️ This comment has been updated with latest results.

@atala-dev
Copy link
Contributor

atala-dev commented Apr 3, 2024

🦙 MegaLinter status: ⚠️ WARNING

Descriptor Linter Files Fixed Errors
✅ ACTION actionlint 1 0
⚠️ MARKDOWN markdownlint 5 90
⚠️ MARKDOWN markdown-link-check 5 10
✅ MARKDOWN markdown-table-formatter 5 0
⚠️ PROTOBUF protolint 5 5
✅ REPOSITORY dustilock yes no
✅ REPOSITORY git_diff yes no
✅ REPOSITORY grype yes no
⚠️ REPOSITORY kics yes 51
✅ REPOSITORY syft yes no
✅ REPOSITORY trivy-sbom yes no
⚠️ REPOSITORY trufflehog yes 1
✅ SQL sql-lint 2 0
⚠️ YAML prettier 2 1
✅ YAML v8r 2 0
✅ YAML yamllint 2 0

See detailed report in MegaLinter reports
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

MegaLinter is graciously provided by OX Security

@shotexa
Copy link

shotexa commented Apr 3, 2024

Why are we not deleting node API and repositories related to credentials in this PR as well? or are you planning to do it in separate PR instead?

@EzequielPostan
Copy link
Contributor Author

I wanted to sync up with you to coordinate the protobuf dependencies first. If we remove the SDK dependency and move the protos to this repo, we can work that here. But the plan is to remove all the parts you are mentioning too (either in this PR, or a follow up one)

This commit deletes the IssueCredentialBatchOperation and
RevokeCredentialsOperation. It does not delete node API, daos nor
repositories related to credentials
This commit deletes even more files related to VC operations. It also
removes some dependencies on the old SDK
@EzequielPostan EzequielPostan force-pushed the ATL-6668-delete-legacy-credentials-operations branch from 58290fb to dd34eae Compare April 15, 2024 02:00
@EzequielPostan EzequielPostan changed the title [WIP] ATL-6668: Delete legacy operation related to credentials refactor(node) Delete legacy operation related to credentials Apr 15, 2024
@EzequielPostan
Copy link
Contributor Author

I moved the protobuf to this repo, also deleted all the dependencies from the SDK except for crypto and identity as we agreed in slack
I believe we can delete even more things in another PR (I would remove the node explorer, which would allow us to remove the authentication stuff too), but lets leave that for the end

@EzequielPostan EzequielPostan marked this pull request as ready for review April 15, 2024 02:05
This commit deletes tables and indexes from VC legacy operations
@EzequielPostan EzequielPostan changed the title refactor(node) Delete legacy operation related to credentials refactor: Delete legacy operation related to credentials Apr 15, 2024
Comment on lines +114 to +122
message AtalaMessage {
oneof message {
AtalaErrorMessage atala_error_message = 9;
}
}

message ConnectionsStatusRequest {
repeated string connection_tokens = 1;
}
Copy link

Choose a reason for hiding this comment

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

Was not this used only by a connector? can you try removing this and see if it compiles?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to add it in order to keep a test related to ClientHelperSpec
I didn't invest enough time to understand the impact of that test. I would need to explore it further

Copy link

Choose a reason for hiding this comment

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

Yeah, it seems like ConnectionsStatusRequest is used in node explorer, but AtalaMessage, this one can very likely be deleted for sure right? it was used by the connector

Comment on lines 321 to 322
IssueCredentialBatchOperationType = 3;
RevokeCredentialsOperationType = 4;
Copy link

Choose a reason for hiding this comment

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

I thought we are removing those credential operations in protobuff as well

Comment on lines 273 to 275
message IssueCredentialBatchOutput {
string batch_id = 1;
}
Copy link

Choose a reason for hiding this comment

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

@@ -0,0 +1,47 @@
// Copyright 2020 Google LLC
Copy link

Choose a reason for hiding this comment

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

Do we need this file? what is this for and where do we use "Status"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was added by the authentication code. Without this, the project fails to compile

Copy link

Choose a reason for hiding this comment

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

I think we can also remove

CREATE DOMAIN public.merkle_root AS BYTEA CONSTRAINT merkle_root_check CHECK (length(VALUE) = 32);

on line 40, Merkle Roots was also only used with credentials

@EzequielPostan
Copy link
Contributor Author

Thank you @shotexa for catching the sql and protobuf parts. I copied the protobuf from the SDK, deleted the node API but forgot the other data. I will try to use the spec protos for node_models.proto

This commit deletes references to legacy VC code in protobuf and sql
definitions
@EzequielPostan
Copy link
Contributor Author

In theory, I addressed the comments

Thank you @shotexa

Copy link

@shotexa shotexa left a comment

Choose a reason for hiding this comment

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

tyvm for your help @EzequielPostan

@EzequielPostan EzequielPostan merged commit 0dc8d9e into open-source-node Apr 17, 2024
4 checks passed
@EzequielPostan EzequielPostan deleted the ATL-6668-delete-legacy-credentials-operations branch April 17, 2024 00:54
milosbackonja added a commit that referenced this pull request May 17, 2024
* refactor: clean up migrations (#830)

* Add everything

Signed-off-by: Shota Jolbordi <[email protected]>

* Remove migration tests

Signed-off-by: Shota Jolbordi <[email protected]>

* add ingnore invalid create option for postres mega linter

Signed-off-by: Shota Jolbordi <[email protected]>

* Try mega linter 7

Signed-off-by: Shota Jolbordi <[email protected]>

* edit megalinter.yaml

Signed-off-by: Shota Jolbordi <[email protected]>

* Disable some linters

Signed-off-by: Shota Jolbordi <[email protected]>

---------

Signed-off-by: Shota Jolbordi <[email protected]>

* refactor(node): remove legacy BE files (#832)

* Start cleaning up docs

Signed-off-by: Shota Jolbordi <[email protected]>

* Remove more files

Signed-off-by: Shota Jolbordi <[email protected]>

* Fix compilation issue

Signed-off-by: Shota Jolbordi <[email protected]>

* Trim docs

Signed-off-by: Shota Jolbordi <[email protected]>

* Fix compilation errors

Signed-off-by: Shota Jolbordi <[email protected]>

* Remove some bitcoin test resources and old docs

Signed-off-by: Shota Jolbordi <[email protected]>

* Lint

Signed-off-by: Shota Jolbordi <[email protected]>

* Fix tests

Signed-off-by: Shota Jolbordi <[email protected]>

---------

Signed-off-by: Shota Jolbordi <[email protected]>

* refactor: Delete legacy operation related to credentials (#831)

* ATL-6668: Delete legacy operation related to credentials

This commit deletes the IssueCredentialBatchOperation and
RevokeCredentialsOperation. It does not delete node API, daos nor
repositories related to credentials

* ATL-6668: Delete more files related to VCs

This commit deletes even more files related to VC operations. It also
removes some dependencies on the old SDK

* ATL-6668: Delete unused tables

This commit deletes tables and indexes from VC legacy operations

* ATL-6668: Fix formatting

* ATL-6668: Address review comments

This commit deletes references to legacy VC code in protobuf and sql
definitions

* refactor(ATL-6924): add support for other keys (#834)

* ATL-6924: Replace users of ECPublicKey

This commit replaces the used of ECPublicKey

* ATL-6924: Simplify models

This commit simplifies models by deleting classes not needed at parsing
time

* ATL-6924: Simplify types

This commit replaces some uses of Array and uses Vector instead to allow
more transparent equality checks

* ATL-6924: Add support for encoding functions

This commit adds support to decode hex strings into Sha256Hash bytes

* ATL-6924: Refactor CryptoUtils and remove SHA256Digest

This commit improves the organization of CryptoUtils methods
It also delete the used of SDK methods related to Sha256 hashing

* refactor(ATL-6926): remove crypto sdk (#838)

* ATL-6926: Remove uses of EC classes

This commit removes most uses of the legacy crypto SDK

* ATL-6926: Fix missing implementations

This commit adds missing implementations for key encoding algorithms

* ATL-6926: Fix tests

* ATL-6926: Clean up code and tests

* ATL-6926: Correct typo and add tests

This commit adds the final tests to validate the cryptography
implementation that replaces the SDK

* refactor(node): Remove prism identity (#840)

* clean up source files, replace DID usages

Signed-off-by: Shota Jolbordi <[email protected]>

* Fix some tests

Signed-off-by: Shota Jolbordi <[email protected]>

* Linting

Signed-off-by: Shota Jolbordi <[email protected]>

* Fix test

Signed-off-by: Shota Jolbordi <[email protected]>

* Fix fromString on DID

Signed-off-by: Shota Jolbordi <[email protected]>

* formatting

Signed-off-by: Shota Jolbordi <[email protected]>

* Address some PR comments

Signed-off-by: Shota Jolbordi <[email protected]>

---------

Signed-off-by: Shota Jolbordi <[email protected]>

* refactor: crypto utils and tests (#841)

* Remove uses of the old SDK

This commit removes the uses of the old SDK and update tests

* Refactor CryptoUtils class

This commit re-organizes and renames methods for better use

* Remove last dependency on old SDK

This commit removes the last dependencies on the old SDK

* docs(ATL-6669): update readme (#833)

* ATL-6669: Update README

This commits updates the README to focus oly on the PRISM node.

* ATL-6669: Add IDE notes in README

This commit adds instructions to load the project in different IDEs.
The commit also removes incorrect sentences

* ATL-6669: Delete old README and complete new one

* ATL-6669: Add pre-commit configuration

This commit adds a pre-commit hook that runs scalafmt and instructions
to configure it

* ATL-6669: Refer to identus configuration

This commit updates the README and refers users to read identus
documentation in order to configure the node.

* ATL-7040: Delete DID based authentication (#843)

This commit deletes the legacy DID based authentication

* refactor(node): build files and add default port for gRPC server (#844)

* Change some build files

Signed-off-by: Shota Jolbordi <[email protected]>

* Rmove unused files

Signed-off-by: Shota Jolbordi <[email protected]>

* Remove unused files

Signed-off-by: Shota Jolbordi <[email protected]>

* Remove outer node folder

Signed-off-by: Shota Jolbordi <[email protected]>

* Revert version name

Signed-off-by: Shota Jolbordi <[email protected]>

* Remove unneeded resolver from the build

Signed-off-by: Shota Jolbordi <[email protected]>

* Update fs2

Signed-off-by: Shota Jolbordi <[email protected]>

* Start adding tests

Signed-off-by: Shota Jolbordi <[email protected]>

* Add more tests

Signed-off-by: Shota Jolbordi <[email protected]>

* Fix 1 failing test

Signed-off-by: Shota Jolbordi <[email protected]>

* Minor adjustment in docs

Signed-off-by: Shota Jolbordi <[email protected]>

* Remove test report aggregation step

Signed-off-by: Shota Jolbordi <[email protected]>

---------

Signed-off-by: Shota Jolbordi <[email protected]>

* fix: Add http scheme support and removed the constraint from public keys table (#848)

* Add http scheme support and removed the contraint from publickeys table

Signed-off-by: mineme0110 <[email protected]>

* ran scalafmt

Signed-off-by: mineme0110 <[email protected]>

---------

Signed-off-by: mineme0110 <[email protected]>

---------

Signed-off-by: Shota Jolbordi <[email protected]>
Signed-off-by: mineme0110 <[email protected]>
Co-authored-by: shotexa <[email protected]>
Co-authored-by: Ezequiel Postan <[email protected]>
Co-authored-by: Shailesh Patil <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants