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

Abstract key storage and signing operations to facilitate different key storage types #291

Conversation

PManaras
Copy link
Contributor

This PR introduces support for implementing different types of keystorages via the newly KeyProvider interface. This interface can be used to retrieve keys from different types of storages, i.e. remote vaults, file-system, database.

The newly created SigningService can be used to implement different types of signature processes. The standard signature process is "LOCAL" meaning the MIW will use keys provided by the keyprovider to sign VP/VC locally.

This PR also adds to new properties to application.yaml (authoritySigningServiceType (defaults to "LOCAL), localSigningKeyStorageType (defaults to DB). These additions are also reflected in the corresponding env-files.

The database entity Wallet was updated and now contains a new field signingServiceType that reflects which wallet uses which signatureServices. The new field is introduced via a new database migration (v3) and sets the signing_service_type to "LOCAL" for all existing wallets. This means: with the standard configuration the MIW will act as before.

this PR includes changes made in the PR on the main-branch

Pre-review checks

Please ensure to do as many of the following checks as possible, before asking for committer review:

This PR is qual to PR285 from a different merge source

@PManaras PManaras changed the title Pr upstream keystorage Abstract key storage and signing operations to facilitate different key storage types Apr 25, 2024
@borisrizov-zf borisrizov-zf self-requested a review April 25, 2024 07:48
Copy link
Contributor

@borisrizov-zf borisrizov-zf left a comment

Choose a reason for hiding this comment

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

Two minor changes needed due to divergence in source branches, after that we will wait for the IP check approval.

CHANGELOG.md Outdated Show resolved Hide resolved
Copy link

sonarcloud bot commented Apr 25, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@nitin-vavdiya
Copy link
Contributor

nitin-vavdiya commented May 27, 2024

License headers need to be updated in changed files

@borisrizov-zf
Copy link
Contributor

@thackerronak @nitin-vavdiya We're ok to rebase this branch and do final testing before we merge.

@nitin-vavdiya
Copy link
Contributor

@thackerronak @nitin-vavdiya We're ok to rebase this branch and do final testing before we merge.

Rebase is done, fixing failing test cases, will let you know once it is ready to merge

@nitin-vavdiya
Copy link
Contributor

@borisrizov-zf
Tested all APIs manually and all tests are also working as expected.
This PR is ready to merge

CHANGELOG.md Outdated
@@ -13,6 +13,82 @@

### Bug Fixes

* add asJwt as query param and fix
Copy link
Contributor

Choose a reason for hiding this comment

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

Where do all of these entries come from?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is merge commit I think after rebase

@@ -33,20 +33,43 @@ public interface WalletKeyRepository extends BaseRepository<WalletKey, Long> {
/**
* Gets by wallet id and algorithm.
*
* @param id the id
* param algorithm the algorithm
* @param id the id param algorithm the algorithm
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like a formatting issue here.

Copy link
Contributor

Choose a reason for hiding this comment

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

fixed with: fcf86af

Copy link

sonarcloud bot commented May 29, 2024

Quality Gate Passed Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
1.5% Duplication on New Code

See analysis details on SonarCloud

@borisrizov-zf borisrizov-zf merged commit ba2f0e8 into eclipse-tractusx:develop Jun 3, 2024
8 checks passed
Copy link

github-actions bot commented Jun 3, 2024

🎉 This PR is included in version 0.5.0-develop.18 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Copy link

github-actions bot commented Jul 5, 2024

🎉 This issue has been resolved in version 0.5.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants