Skip to content

Commit

Permalink
Moved from Accept to Accept-Encoding header
Browse files Browse the repository at this point in the history
The secrets batch endpoint used to look at the Accept
header to determine if it should base64-encode the response.
Now it looks at the Accept-Encoding header. This change
falls in line better with the intended uses of both
headers
  • Loading branch information
telday committed Mar 11, 2021
1 parent e9cf7cb commit a4820f9
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 5 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- When a user checks permissions of a non-existing role or a non-existing resource, Conjur now audits a failure message.
[cyberark/conjur#2059](https://github.com/cyberark/conjur/issues/2059)

### Changed
- The secrets batch retrieval endpoint now refers to the `Accept-Encoding` header rather than `Accept` to determine the response encoding
[cyberark/conjur#2065](https://github.com/cyberark/conjur/pull/2065)

## [1.11.4] - 2021-03-09

### Security
Expand Down
9 changes: 7 additions & 2 deletions app/controllers/secrets_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,13 @@ def get_secret_from_variable(variable)
raise Exceptions::RecordNotFound, variable.resource_id unless secret

secret_value = secret.value
accepts_base64 = String(request.headers['Accept']).casecmp?('base64')
accepts_base64 ? Base64.encode64(secret_value) : secret_value
accepts_base64 = String(request.headers['Accept-Encoding']).casecmp?('base64')
if accepts_base64
response.set_header("Content-Encoding", "base64")
Base64.encode64(secret_value)
else
secret_value
end
end

def audit_fetch resource, version: nil
Expand Down
7 changes: 4 additions & 3 deletions cucumber/api/features/secrets_batch.feature
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,14 @@ Feature: Batch retrieval of secrets
Scenario: Returns the correct result for binary secrets
Given I create a binary secret value for resource "cucumber:variable:secret3"
And I add the secret value "v2" to the resource "cucumber:variable:secret2"
And I set the "Accept" header to "base64"
And I set the "Accept-Encoding" header to "base64"
When I GET "/secrets?variable_ids=cucumber:variable:secret3,cucumber:variable:secret2"
Then the binary data is preserved for "cucumber:variable:secret3"
And the content encoding is "base64"

Scenario: Returns the correct result for binary secrets
Given I create a binary secret value for resource "cucumber:variable:secret3"
And I set the "Accept" header to "Base64"
And I set the "Accept-Encoding" header to "Base64"
When I GET "/secrets?variable_ids=cucumber:variable:secret3"
Then the binary data is preserved for "cucumber:variable:secret3"

Expand All @@ -115,7 +116,7 @@ Feature: Batch retrieval of secrets
When I GET "/secrets?variable_ids=cucumber:variable:secret3,cucumber:variable:secret2"
Then the HTTP response status code is 500

Scenario: Omit the Accept header entirely from batch secrets request
Scenario: Omit the Accept-Encoding header entirely from batch secrets request
Given I add the secret value "v2" to the resource "cucumber:variable:secret2"
When I GET "/secrets?variable_ids=cucumber:variable:secret2" with no default headers
Then the JSON should be:
Expand Down
4 changes: 4 additions & 0 deletions cucumber/api/features/step_definitions/response_steps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@
expect(@result).to eq(@value)
end

Then(/^the content encoding is "([^"]*)"/) do |encoding|
expect(@content_encoding).to eq(encoding)
end

Then(/^the binary data is preserved for "([^"]*)"$/) do |resource_id|
data = Base64.decode64(@result[resource_id])
expect(data).to eq(@value)
Expand Down
1 change: 1 addition & 0 deletions cucumber/api/features/support/rest_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ def set_result result
@http_status = result.code

@content_type = result.headers[:content_type]
@content_encoding = result.headers[:content_encoding]
if /^application\/json/.match?(@content_type)
@result = JSON.parse(result)
@response_api_key = @result['api_key'] if @result.is_a?(Hash)
Expand Down

0 comments on commit a4820f9

Please sign in to comment.