conjur icon indicating copy to clipboard operation
conjur copied to clipboard

POC: Batch retrieval with POST and JSON body

Open john-odonnell opened this issue 2 years ago • 4 comments

Desired Outcome

Inspired by a Salesforce case where users encountered a 414 Request-URI Too Large when batch-retrieving 75+ variables.

Implemented Changes

Accept POST requests to /secrets endpoint, where variable IDs are passed as part of a JSON response body instead of as a query parameter.

Connected Issue/Story

Resolves #[relevant GitHub issue(s), e.g. 76]

CyberArk internal issue ID: [insert issue ID]

Definition of Done

At least 1 todo must be completed in the sections below for the PR to be merged.

Changelog

  • [ ] The CHANGELOG has been updated, or
  • [ ] This PR does not include user-facing changes and doesn't require a CHANGELOG update

Test coverage

  • [ ] This PR includes new unit and integration tests to go with the code changes, or
  • [ ] The changes in this PR do not require tests

Documentation

  • [ ] Docs (e.g. READMEs) were updated in this PR
  • [ ] A follow-up issue to update official docs has been filed here: [insert issue ID]
  • [ ] This PR does not require updating any documentation

Behavior

  • [ ] This PR changes product behavior and has been reviewed by a PO, or
  • [ ] These changes are part of a larger initiative that will be reviewed later, or
  • [ ] No behavior was changed with this PR

Security

  • [ ] Security architect has reviewed the changes in this PR,
  • [ ] These changes are part of a larger initiative with a separate security review, or
  • [ ] There are no security aspects to these changes

john-odonnell avatar Feb 01 '23 19:02 john-odonnell

Code Climate has analyzed commit febb867c and detected 2 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 2

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 90.1% (-1.5% change).

View more on Code Climate.

codeclimate[bot] avatar Feb 01 '23 20:02 codeclimate[bot]

@john-odonnell, a couple of questions:

  1. What does the JSON request body look like? How does the method handle a malformed JSON document (missing keys, etc.)?
  2. What does the response JSON look like?
  3. What happens if the requestor has access to only a subset of the variable values? What happens if some of the variables have not been instantiated with a secret value?

jvanderhoof avatar Feb 02 '23 18:02 jvanderhoof

Thanks for the questions, @jvanderhoof. This is related to ONYX-31473, where I've written a more detailed analysis of the problem. Essentially, the 414 Request-URI Too Large error is caused by NGINX configuration, and not directly by Conjur. I wonder if it really warrants any changes to the API, but I was simply demonstrating that we reasonably could.

What does the JSON request body look like?

For convenience, the changes I've made will process a JSON request intentionally formatted to mirror the expected query parameter:

{"variable_ids":["account:kind:a",...,"account:kind:b"]}

How does the method handle a malformed JSON document (missing keys, etc.)?

Good question. I'm definitely short on test cases here. I'll add some this afternoon.

What does the response JSON look like? What happens if the requestor has access to only a subset of the variable values? What happens if some of the variables have not been instantiated with a secret value?

This behavior should mimic the existing behavior of batch secret retrieval. Batch retrieval with a GET and a POST both direct to secrets#batch, and the only thing I've updated is query parameter/request body processing. That said, permission issues and value-less variables cause a 404, and the response body should look like this:

{
  "account:kind:a": "<value>",
  ...
  "account:kind:b": "<value"
}

john-odonnell avatar Feb 03 '23 15:02 john-odonnell

Thanks @john-odonnell! I'll take a look at the above Jira ticket 😄.

jvanderhoof avatar Feb 07 '23 16:02 jvanderhoof