-
Notifications
You must be signed in to change notification settings - Fork 283
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
feat(plugin-keychain-memory): add REST API endpoint implementations #2893
Merged
petermetz
merged 1 commit into
hyperledger-cacti:main
from
petermetz:feat-keychain-memory-rest-api-endpoints
Dec 6, 2023
Merged
feat(plugin-keychain-memory): add REST API endpoint implementations #2893
petermetz
merged 1 commit into
hyperledger-cacti:main
from
petermetz:feat-keychain-memory-rest-api-endpoints
Dec 6, 2023
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
petermetz
requested review from
takeutak,
izuru0,
jagpreetsinghsasan,
VRamakrishna,
sandeepnRES and
outSH
as code owners
November 16, 2023 04:41
jagpreetsinghsasan
approved these changes
Nov 16, 2023
outSH
requested changes
Nov 16, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some comments
...us-plugin-keychain-memory/src/test/typescript/unit/plugin-keychain-memory-api-client.test.ts
Outdated
Show resolved
Hide resolved
outSH
requested changes
Nov 16, 2023
...plugin-keychain-memory/src/main/typescript/web-services/delete-keychain-entry-endpoint-v1.ts
Outdated
Show resolved
Hide resolved
**BREAKING CHANGE**: Some of the schema elements that were redundant got removed from the openapi.json spec of the memory plugin. The same schemas are now being pulled from the core-api package. In this sense there is no breaking change because these two schemas were already equivalent anyway, but the fact is the openapi.json had schema elements removed from it anyway so this is something that people need to be aware of as they upgrade to v2.0.0 in the near future. 1. Added the REST API endpoints to the plugin so that it is accessible over HTTP for testing/demo purposes as necessary. 2. Also refactored the openapi.json spec file so that it pulls the schema definitions from the core-api package the same way as the keychain-memory-wasm plugin is doing already. 3. Migrated the test API client test case to Jest from tape 4. Refactored the `handleRequest` methods of the endpoint classes so that they perform the error handling with respect to the HTTP status codes being set correctly (sanitization is pending because I have to send a separate PR for that to the cactus-core package) Special thanks to @outSH for the suggestions to improve things! Signed-off-by: Peter Somogyvari <[email protected]>
petermetz
force-pushed
the
feat-keychain-memory-rest-api-endpoints
branch
from
December 5, 2023 22:15
8d0b9ac
to
48f2209
Compare
petermetz
referenced
this pull request
in petermetz/cacti
Dec 5, 2023
1. This is a security fix so that the exception serialization does not accidentally XSS anybody who is looking at crash logs through some admin GUI that is designed to show logs that are considered trusted. 2. The yarn.lock file seems to have gotten out of date by accident again so I'm also sneaking in that as an update here just to get the fix in ASAP and without burning too much on CI execution costs. Related discussion about `1)` can be seen at this other pull request: https://github.com/hyperledger/cacti/pull/2893 Signed-off-by: Peter Somogyvari <[email protected]>
5 tasks
petermetz
referenced
this pull request
in petermetz/cacti
Dec 6, 2023
1. This is a security fix so that the exception serialization does not accidentally XSS anybody who is looking at crash logs through some admin GUI that is designed to show logs that are considered trusted. 2. The yarn.lock file seems to have gotten out of date by accident again so I'm also sneaking in that as an update here just to get the fix in ASAP and without burning too much on CI execution costs. Related discussion about `1)` can be seen at this other pull request: https://github.com/hyperledger/cacti/pull/2893 Signed-off-by: Peter Somogyvari <[email protected]> (cherry picked from commit 7cf4a73)
outSH
approved these changes
Dec 6, 2023
petermetz
referenced
this pull request
in petermetz/cacti
Dec 7, 2023
1. This is a security fix so that the exception serialization does not accidentally XSS anybody who is looking at crash logs through some admin GUI that is designed to show logs that are considered trusted. 2. The yarn.lock file seems to have gotten out of date by accident again so I'm also sneaking in that as an update here just to get the fix in ASAP and without burning too much on CI execution costs. Related discussion about `1)` can be seen at this other pull request: https://github.com/hyperledger/cacti/pull/2893 Signed-off-by: Peter Somogyvari <[email protected]> (cherry picked from commit 7cf4a73)
petermetz
referenced
this pull request
in petermetz/cacti
Dec 15, 2023
1. This is a security fix so that the exception serialization does not accidentally XSS anybody who is looking at crash logs through some admin GUI that is designed to show logs that are considered trusted. 2. The yarn.lock file seems to have gotten out of date by accident again so I'm also sneaking in that as an update here just to get the fix in ASAP and without burning too much on CI execution costs. Related discussion about `1)` can be seen at this other pull request: https://github.com/hyperledger/cacti/pull/2893 Signed-off-by: Peter Somogyvari <[email protected]> (cherry picked from commit 7cf4a73)
petermetz
referenced
this pull request
in petermetz/cacti
Dec 21, 2023
1. This is a security fix so that the exception serialization does not accidentally XSS anybody who is looking at crash logs through some admin GUI that is designed to show logs that are considered trusted. Related discussion about `1)` can be seen at this other pull request: https://github.com/hyperledger/cacti/pull/2893 Signed-off-by: Peter Somogyvari <[email protected]>
petermetz
referenced
this pull request
in petermetz/cacti
Jan 4, 2024
1. This is a security fix so that the exception serialization does not accidentally XSS anybody who is looking at crash logs through some admin GUI that is designed to show logs that are considered trusted. Related discussion about `1)` can be seen at this other pull request: https://github.com/hyperledger/cacti/pull/2893 Signed-off-by: Peter Somogyvari <[email protected]>
petermetz
referenced
this pull request
in petermetz/cacti
Jan 9, 2024
1. This is a security fix so that the exception serialization does not accidentally XSS anybody who is looking at crash logs through some admin GUI that is designed to show logs that are considered trusted. 2. The yarn.lock file seems to have gotten out of date by accident again so I'm also sneaking in that as an update here just to get the fix in ASAP and without burning too much on CI execution costs. Related discussion about `1)` can be seen at this other pull request: https://github.com/hyperledger/cacti/pull/2893 Signed-off-by: Peter Somogyvari <[email protected]> (cherry picked from commit 7cf4a73)
petermetz
referenced
this pull request
in petermetz/cacti
Jan 17, 2024
1. This is a security fix so that the exception serialization does not accidentally XSS anybody who is looking at crash logs through some admin GUI that is designed to show logs that are considered trusted. Related discussion about `1)` can be seen at this other pull request: https://github.com/hyperledger/cacti/pull/2893 Signed-off-by: Peter Somogyvari <[email protected]>
petermetz
referenced
this pull request
Jan 17, 2024
1. This is a security fix so that the exception serialization does not accidentally XSS anybody who is looking at crash logs through some admin GUI that is designed to show logs that are considered trusted. Related discussion about `1)` can be seen at this other pull request: https://github.com/hyperledger/cacti/pull/2893 Signed-off-by: Peter Somogyvari <[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.
BREAKING CHANGE: Some of the schema elements that were redundant
got removed from the openapi.json spec of the memory plugin.
The same schemas are now being pulled from the core-api package.
In this sense there is no breaking change because these two
schemas were already equivalent anyway, but the fact is
the openapi.json had schema elements removed from it anyway
so this is something that people need to be aware of as they
upgrade to v2.0.0 in the near future.
HTTP for testing/demo purposes as necessary.
definitions from the core-api package the same way as the
keychain-memory-wasm plugin is doing already.
handleRequest
methods of the endpoint classes so thatthey perform the error handling with respect to the HTTP status codes being
set correctly (sanitization is pending because I have to send a separate
PR for that to the cactus-core package)
Special thanks to @outSH for the suggestions to improve things!
Signed-off-by: Peter Somogyvari [email protected]
Pull Request Requirements
upstream/main
branch and squashed into single commit to help maintainers review it more efficient and to avoid spaghetti git commit graphs that obfuscate which commit did exactly what change, when and, why.-s
flag when usinggit commit
command. You may refer to this link for more information.Character Limit
A Must Read for Beginners
For rebasing and squashing, here's a must read guide for beginners.