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

fix(cmd-api-server): OpenAPI spec validation missing from plugin REST endpoints #847

Closed
petermetz opened this issue Apr 23, 2021 · 4 comments · Fixed by #1294
Closed

fix(cmd-api-server): OpenAPI spec validation missing from plugin REST endpoints #847

petermetz opened this issue Apr 23, 2021 · 4 comments · Fixed by #1294
Assignees
Labels
API_Server bug Something isn't working Core_API Changes related to the Core API Package good-first-issue Good for newcomers good-first-issue-200-intermediate Hacktoberfest Hacktoberfest participants are welcome to take a stab at issues marked with this label.

Comments

@petermetz
Copy link
Member

Describe the bug

Right now the constraints we define for request/response objects are not checked when a request comes in. (except for the api-server's built in endpoints).

To Reproduce

Send an invalid request to some plugin endpoint that is invalid in a certain way and watch how it crashes the plugin resulting in internal server error vs. bad request.

Expected behavior

Fail with bad request even before the plugin's back-end code is reached if a request is sent that does not comply with the open api specs defined for that endpoint.

Logs/Stack traces

N/A

Screenshots

N/A

Cloud provider or hardware configuration:

N/A

Operating system name, version, build:

N/A

Hyperledger Cactus release version or commit (git rev-parse --short HEAD):

main @ today

Hyperledger Cactus Plugins/Connectors Used

All of them.

Additional context

The mechanics of this are very simple and there's also a working example in the code of the api-server.ts file, but what needs to be given some more thought prior to jumping in is how to do this within the confines of the plugin architecture.
More specifically one of the questions that needs answering prior to actual work is who should be responsible for registering the validation middleware? Is it the api-server when hooking up the plugin's web services or should we delegate this entirely to the plugin instance itself?

My recommendation is below (but this could turn out to be a bad idea so be ready for the need to do some refactoring once we see how it plays out)

The interface of the web service plugins should be extended with a getOpenApiSpecs() method that returns an OpenAPI v3 spec document (see the openapi-types package)
Then the api-server calls up this method when it's installing the web services of the plugin, obtains the open api specs and creates a validator instance from it, hooks that middleware in to the api-server's express JS object.

cc: @takeutak @sfuji822 @hartm @jonathan-m-hamilton @AzaharaC @jordigiam @kikoncuo @jagpreetsinghsasan @arnab-roy @TonyRowntree

@petermetz petermetz added bug Something isn't working good-first-issue Good for newcomers API_Server Hacktoberfest Hacktoberfest participants are welcome to take a stab at issues marked with this label. Core_API Changes related to the Core API Package good-first-issue-200-intermediate labels Apr 23, 2021
@TonyRowntree TonyRowntree self-assigned this Apr 27, 2021
@TonyRowntree
Copy link
Member

hey @petermetz is this done?

@elenaizaguirre
Copy link
Contributor

@petermetz, can I work with this?

@TonyRowntree TonyRowntree removed their assignment Jun 14, 2021
@petermetz
Copy link
Member Author

hey @petermetz is this done?

@TonyRowntree Not yet, no.

@petermetz
Copy link
Member Author

@petermetz, can I work with this?

@elenaizaguirre Yup! Feel free to pick it up!

elenaizaguirre added a commit to elenaizaguirre/cactus that referenced this issue Jul 14, 2021
add missing validation from plugin REST endpoints

fixes hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
elenaizaguirre added a commit to elenaizaguirre/cactus that referenced this issue Jul 14, 2021
add missing validation from plugin REST endpoints

fixes hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
elenaizaguirre added a commit to elenaizaguirre/cactus that referenced this issue Aug 4, 2021
add missing validation from plugin REST endpoints

fixes hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
elenaizaguirre added a commit to elenaizaguirre/cactus that referenced this issue Aug 4, 2021
add missing validation from plugin REST endpoints

fixes hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
elenaizaguirre added a commit to elenaizaguirre/cactus that referenced this issue Aug 4, 2021
add missing validation from plugin REST endpoints

fixes hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
elenaizaguirre added a commit to elenaizaguirre/cactus that referenced this issue Aug 10, 2021
add missing validation from plugin REST endpoints

fixes hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
petermetz pushed a commit to elenaizaguirre/cactus that referenced this issue Aug 11, 2021
add missing validation from plugin REST endpoints

fixes hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
elenaizaguirre added a commit to elenaizaguirre/cactus that referenced this issue Aug 19, 2021
add missing validation from plugin REST endpoints

fixes hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
elenaizaguirre added a commit to elenaizaguirre/cactus that referenced this issue Aug 20, 2021
add missing validation for plugin REST endpoints

fixes hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
elenaizaguirre added a commit to elenaizaguirre/cactus that referenced this issue Aug 20, 2021
add missing validation for plugin REST endpoints

fixes hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
elenaizaguirre added a commit to elenaizaguirre/cactus that referenced this issue Aug 31, 2021
add missing validation for plugin REST endpoints

fixes hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
elenaizaguirre added a commit to elenaizaguirre/cactus that referenced this issue Aug 31, 2021
add missing validation for plugin REST endpoints

fixes hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
elenaizaguirre added a commit to elenaizaguirre/cactus that referenced this issue Sep 6, 2021
This method allows us to re-use the API server's internal
mechanisms to configure the OpenAPI spec validation in
test cases where we cannot depend on the API server itself
due to circular dependencies.
So this method is designed to be used both by the API server
and the test cases at the same time.

Closes: hyperledger#847
elenaizaguirre added a commit to elenaizaguirre/cactus that referenced this issue Sep 6, 2021
This method allows us to re-use the API server's internal
mechanisms to configure the OpenAPI spec validation in
test cases where we cannot depend on the API server itself
due to circular dependencies.
So this method is designed to be used both by the API server
and the test cases at the same time.

Closes: hyperledger#847
Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
elenaizaguirre added a commit to elenaizaguirre/cactus that referenced this issue Sep 7, 2021
This method allows us to re-use the API server's internal
mechanisms to configure the OpenAPI spec validation in
test cases where we cannot depend on the API server itself
due to circular dependencies.
So this method is designed to be used both by the API server
and the test cases at the same time.

Closes: hyperledger#847
Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
petermetz pushed a commit that referenced this issue Oct 5, 2021
Includes tests for endpoints deployContractGoSourceV1,
deployContractV1 and runTransactionV1, each of them with
test cases:
  - Right request
  - Request including an invalid parameter
  - Request without a required parameter

Closes #1295

Relationed with #847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
elenaizaguirre added a commit to elenaizaguirre/cactus that referenced this issue Oct 6, 2021
Includes tests for endpoints setKeychainEntry, getKeychainEntryV1,
hasKeychainEntryV1 and deleteKeychainEntryV1, each of them with
test cases:
  - Right request
  - Request including an invalid parameter
  - Request without a required parameter

Modifies cactus-core-api definitions for GetKeychainEntryRequestV1
and DeleteKeychainEntryRequest adding to them the option
additionalProperties: false, and removes this option from
GetKeychainEntryResponseV1 and DeleteKeychainEntryResponseV1

Relationed with hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
elenaizaguirre added a commit to elenaizaguirre/cactus that referenced this issue Oct 6, 2021
Includes tests for endpoints setKeychainEntry, getKeychainEntryV1,
hasKeychainEntryV1 and deleteKeychainEntryV1, each of them with
test cases:
  - Right request
  - Request including an invalid parameter
  - Request without a required parameter

Modifies cactus-core-api definitions for GetKeychainEntryRequestV1
and DeleteKeychainEntryRequest adding to them the option
additionalProperties: false, and removes this option from
GetKeychainEntryResponseV1 and DeleteKeychainEntryResponseV1

Relationed with hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
elenaizaguirre added a commit to elenaizaguirre/cactus that referenced this issue Oct 6, 2021
Includes tests for endpoints setKeychainEntry, getKeychainEntryV1,
hasKeychainEntryV1 and deleteKeychainEntryV1, each of them with
test cases:
  - Right request
  - Request including an invalid parameter
  - Request without a required parameter

Modifies cactus-core-api definitions for GetKeychainEntryRequestV1
and DeleteKeychainEntryRequest adding to them the option
additionalProperties: false, and removes this option from
GetKeychainEntryResponseV1 and DeleteKeychainEntryResponseV1

Relationed with hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
elenaizaguirre added a commit to elenaizaguirre/cactus that referenced this issue Oct 13, 2021
Includes tests for endpoints setKeychainEntry, getKeychainEntryV1,
hasKeychainEntryV1 and deleteKeychainEntryV1, each of them with
test cases:
  - Right request
  - Request including an invalid parameter
  - Request without a required parameter

Relationed with hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
elenaizaguirre added a commit to elenaizaguirre/cactus that referenced this issue Oct 13, 2021
Includes tests for endpoints setKeychainEntry, getKeychainEntryV1,
hasKeychainEntryV1 and deleteKeychainEntryV1, each of them with
test cases:
  - Right request
  - Request including an invalid parameter
  - Request without a required parameter

Relationed with hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
elenaizaguirre added a commit to elenaizaguirre/cactus that referenced this issue Oct 13, 2021
Includes tests for endpoints setKeychainEntry, getKeychainEntryV1,
hasKeychainEntryV1 and deleteKeychainEntryV1, each of them with
test cases:
  - Right request
  - Request including an invalid parameter
  - Request without a required parameter

Relationed with hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
elenaizaguirre added a commit to elenaizaguirre/cactus that referenced this issue Oct 13, 2021
Includes tests for endpoints setKeychainEntry, getKeychainEntryV1,
hasKeychainEntryV1 and deleteKeychainEntryV1, each of them with
test cases:
  - Right request
  - Request including an invalid parameter
  - Request without a required parameter

Relationed with hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
elenaizaguirre added a commit to elenaizaguirre/cactus that referenced this issue Oct 20, 2021
Includes tests for endpoints setKeychainEntry, getKeychainEntryV1,
hasKeychainEntryV1 and deleteKeychainEntryV1, each of them with
test cases:
  - Right request
  - Request including an invalid parameter
  - Request without a required parameter

Relationed with hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
petermetz pushed a commit that referenced this issue Oct 20, 2021
Includes tests for endpoints setKeychainEntry, getKeychainEntryV1,
hasKeychainEntryV1 and deleteKeychainEntryV1, each of them with
test cases:
  - Right request
  - Request including an invalid parameter
  - Request without a required parameter

Relationed with #847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
Leeyoungone pushed a commit to Leeyoungone/cactus that referenced this issue Oct 26, 2021
Includes tests for endpoints setKeychainEntry, getKeychainEntryV1,
hasKeychainEntryV1 and deleteKeychainEntryV1, each of them with
test cases:
  - Right request
  - Request including an invalid parameter
  - Request without a required parameter

Relationed with hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
elenaizaguirre added a commit to elenaizaguirre/cactus that referenced this issue Nov 5, 2021
Includes tests for endpoints setKeychainEntry, getKeychainEntryV1,
hasKeychainEntryV1 and deleteKeychainEntryV1, each of them with
test cases:
  - Right request
  - Request including an invalid parameter
  - Request without a required parameter

Relationed with hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
elenaizaguirre added a commit to elenaizaguirre/cactus that referenced this issue Nov 5, 2021
Includes tests for endpoints setKeychainEntry, getKeychainEntryV1,
hasKeychainEntryV1 and deleteKeychainEntryV1, each of them with
test cases:
  - Right request
  - Request including an invalid parameter
  - Request without a required parameter

Relationed with hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
elenaizaguirre added a commit to elenaizaguirre/cactus that referenced this issue Nov 5, 2021
Includes tests for endpoints setKeychainEntry, getKeychainEntryV1,
hasKeychainEntryV1 and deleteKeychainEntryV1, each of them with
test cases:
  - Right request
  - Request including an invalid parameter
  - Request without a required parameter

Relationed with hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
elenaizaguirre added a commit to elenaizaguirre/cactus that referenced this issue Nov 5, 2021
Includes tests for endpoints setKeychainEntry, getKeychainEntryV1,
hasKeychainEntryV1 and deleteKeychainEntryV1, each of them with
test cases:
  - Right request
  - Request including an invalid parameter
  - Request without a required parameter

Relationed with hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
elenaizaguirre added a commit to elenaizaguirre/cactus that referenced this issue Nov 8, 2021
Includes tests for endpoints setKeychainEntry, getKeychainEntryV1,
hasKeychainEntryV1 and deleteKeychainEntryV1, each of them with
test cases:
  - Right request
  - Request including an invalid parameter
  - Request without a required parameter

Relationed with hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
petermetz pushed a commit that referenced this issue Nov 9, 2021
Includes tests for endpoints setKeychainEntry, getKeychainEntryV1,
hasKeychainEntryV1 and deleteKeychainEntryV1, each of them with
test cases:
  - Right request
  - Request including an invalid parameter
  - Request without a required parameter

Relationed with #847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
RafaelAPB pushed a commit to RafaelAPB/blockchain-integration-framework that referenced this issue Mar 9, 2022
This method allows us to re-use the API server's internal
mechanisms to configure the OpenAPI spec validation in
test cases where we cannot depend on the API server itself
due to circular dependencies.
So this method is designed to be used both by the API server
and the test cases at the same time.

Relationed with hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
RafaelAPB pushed a commit to RafaelAPB/blockchain-integration-framework that referenced this issue Mar 9, 2022
Includes tests for endpoints deployContractSolBytecodeV1,
invokeContractV1 and runTransactionV1, each of them
with test cases:
  - Right request
  - Request including an invalid parameter
  - Request without a required parameter

Closes hyperledger#1286

Relationed with hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
RafaelAPB pushed a commit to RafaelAPB/blockchain-integration-framework that referenced this issue Mar 9, 2022
Includes tests for endpoints deployContractV1,
invokeContractV1 and runTransactionV1, each of them
with test cases:
  - Right request
  - Request including an invalid parameter
  - Request without a required parameter

Closes hyperledger#1288

Relationed with hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
RafaelAPB pushed a commit to RafaelAPB/blockchain-integration-framework that referenced this issue Mar 9, 2022
Includes tests for endpoints getConsortiumJwsV1 and
getNodeJwsV1, each one of them with test cases:
  - Right request
  - Sending an invalid parameter

closes hyperledger#1297

relationed with hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
RafaelAPB pushed a commit to RafaelAPB/blockchain-integration-framework that referenced this issue Mar 9, 2022
Includes tests for endpoints setKeychainEntryV1,
getKeychainEntryV1, deleteKeychainEntryV1 and
hasKeychainEntryV1, each of them, when appropiate,
with test cases:
  - Right request
  - Request including an invalid parameter
  - Request without a required parameter

Closes hyperledger#1329

Relationed with hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
RafaelAPB pushed a commit to RafaelAPB/blockchain-integration-framework that referenced this issue Mar 9, 2022
Changes endpoints getSingleStatus and getStatus in htlc-eth-besu
and htlc-eth-besu-erc20 which now are of type post and defines
for that new request schemas.

Includes tests for all endpoints in besu, htlc-eth-besu and
htlc-eth-besu-erc20, each of them with test cases:
  - Right request
  - Request including an invalid parameter
  - Request without a required parameter

Closes hyperledger#1291

Relationed with hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
RafaelAPB pushed a commit to RafaelAPB/blockchain-integration-framework that referenced this issue Mar 9, 2022
Includes test for endpoint runTransactionV1
with test cases:
  - Right request
  - Request including an invalid parameter
  - Request without a required parameter

Closes hyperledger#1331

Related with hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
RafaelAPB pushed a commit to RafaelAPB/blockchain-integration-framework that referenced this issue Mar 9, 2022
Includes tests for endpoints deployContractGoSourceV1,
deployContractV1 and runTransactionV1, each of them with
test cases:
  - Right request
  - Request including an invalid parameter
  - Request without a required parameter

Closes hyperledger#1295

Relationed with hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
RafaelAPB pushed a commit to RafaelAPB/blockchain-integration-framework that referenced this issue Mar 9, 2022
Includes tests for endpoints setKeychainEntry, getKeychainEntryV1,
hasKeychainEntryV1 and deleteKeychainEntryV1, each of them with
test cases:
  - Right request
  - Request including an invalid parameter
  - Request without a required parameter

Relationed with hyperledger#847

Signed-off-by: Elena Izaguirre <e.izaguirre.equiza@accenture.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API_Server bug Something isn't working Core_API Changes related to the Core API Package good-first-issue Good for newcomers good-first-issue-200-intermediate Hacktoberfest Hacktoberfest participants are welcome to take a stab at issues marked with this label.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants