Merge pull request #2124 from cyberark/change-return-code

Change return code on fail to encode batch secrets

CHANGELOG.md

@@ -11,6 +11,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
   non-alpha-numeric passwords sent via stdin raised an error.
   [cyberark/conjur#2083](https://github.com/cyberark/conjur/issues/2083)
 
+### Changed
+- The batch secret retrieval endpoint now returns a 406 Not Acceptable instead
+  of a 500 error when a secret with incompatible encoding is requested.
+  [cyberark/conjur#2124](https://github.com/cyberark/conjur/pull/2124)
+
 ### Security
 - Upgrade github-pages in docs/Gemfile to resolve CVE-2021-28834 in kramdown dependency [cyberark/conjur#2099](https://github.com/cyberark/conjur/issues/2099)
 - Bump `cyberark/ubi-ruby-fips` from 1.0.1 to 1.0.2 to address CVE-2021-20305.

app/controllers/application_controller.rb

@@ -62,6 +62,7 @@ class UnprocessableEntity < RuntimeError
   rescue_from ArgumentError, with: :argument_error
   rescue_from ActionController::ParameterMissing, with: :argument_error
   rescue_from UnprocessableEntity, with: :unprocessable_entity
+  rescue_from Errors::Conjur::BadSecretEncoding, with: :bad_secret_encoding
 
   around_action :run_with_transaction
 
@@ -159,10 +160,7 @@ def validation_failed e
   def policy_invalid e
     logger.debug("#{e}\n#{e.backtrace.join("\n")}")
 
-    error = {
-      code: "policy_invalid",
-      message: e.message
-    }
+    error = { code: "policy_invalid", message: e.message }
 
     if e.instance_of?(Conjur::PolicyParser::Invalid)
       error[:innererror] = {
@@ -173,9 +171,7 @@ def policy_invalid e
       }
     end
 
-    render(json: {
-      error: error
-    }, status: :unprocessable_entity)
+    render(json: { error: error }, status: :unprocessable_entity)
   end
 
   def argument_error e
@@ -225,6 +221,16 @@ def unprocessable_entity e
     }, status: :unprocessable_entity)
   end
 
+  def bad_secret_encoding e
+    logger.debug("#{e}\n#{e.backtrace.join("\n")}")
+    render(json: {
+      error: {
+        code: :not_acceptable,
+        message: e.message
+      }
+    }, status: :not_acceptable)
+  end
+
   def unauthorized e
     logger.debug("#{e}\n#{e.backtrace.join("\n")}")
     if e.return_message_in_response

app/domain/errors.rb

@@ -26,7 +26,7 @@ module Conjur
     )
 
     BadSecretEncoding = ::Util::TrackableErrorClass.new(
-      msg: "Issue encoding secret into JSON format, try including 'Accept: base64' " \
+      msg: "Issue encoding secret into JSON format, try including 'Accept-Encoding: base64' " \
           "header in request.",
       code: "CONJ00074E"
     )

cucumber/api/features/secrets_batch.feature

@@ -119,13 +119,13 @@ 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"
     When I GET "/secrets?variable_ids=cucumber:variable:secret3"
-    Then the HTTP response status code is 500
+    Then the HTTP response status code is 406
 
   Scenario: Raises error on binary secret with no annotation
     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"
     When I GET "/secrets?variable_ids=cucumber:variable:secret3,cucumber:variable:secret2"
-    Then the HTTP response status code is 500
+    Then the HTTP response status code is 406
 
   Scenario: Omit the Accept-Encoding header entirely from batch secrets request
     Given I add the secret value "v2" to the resource "cucumber:variable:secret2"