Handle HTTP error codes when fetching jwks data

Co-authored-by: Shuli Finkelstein <64148158+shulifink@users.noreply.github.com>
cyberark · Jan 25, 2022 · ae35588 · ae35588
1 parent e6b6ced
commit ae35588
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 79 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Nothing should go in this section, please add to the latest unreleased version
   (and update the corresponding date), or add a new version.
 
-## [1.16.0] - 2022-01-19
+## [1.16.0] - 2022-01-25
 
 ### Added
 - Added the ability to fetch signing keys from JWKS endpoints that use a self-signed
@@ -33,6 +33,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
   [#2447](https://github.com/cyberark/conjur/pull/2447)
   [#2437](https://github.com/cyberark/conjur/pull/2437))
 
+### Changed
+- Proper error message appears when JWT Authenticator gets HTTP code error
+  while trying to fetch JWKS data from `jwks-uri` [#2474](https://github.com/cyberark/conjur/pull/2474)
+
 ## [1.15.1] - 2022-01-12
 
 ### Changed

diff --git a/app/domain/authentication/authn_jwt/signing_key/create_jwks_from_http_response.rb b/app/domain/authentication/authn_jwt/signing_key/create_jwks_from_http_response.rb
@@ -10,19 +10,19 @@ module SigningKey
         inputs: %i[http_response]
       ) do
         def call
-          validate_response_exists
-          validate_response_has_a_body
+          validate_response_success
           create_jwks_from_http_response
         end
 
         private
 
-        def validate_response_exists
-          raise Errors::Authentication::AuthnJwt::MissingHttpResponse if @http_response.blank?
-        end
-
-        def validate_response_has_a_body
-          raise Errors::Authentication::AuthnJwt::InvalidHttpResponseFormat unless @http_response.respond_to?(:body)
+        def validate_response_success
+          @http_response.value
+        rescue => e
+          raise Errors::Authentication::AuthnJwt::FailedToFetchJwksData.new(
+            @http_response.uri,
+            e.inspect
+          )
         end
 
         def create_jwks_from_http_response

diff --git a/app/domain/errors.rb b/app/domain/errors.rb
@@ -443,21 +443,11 @@ module AuthnJwt
         code: "CONJ00093E"
       )
 
-      MissingHttpResponse = ::Util::TrackableErrorClass.new(
-        msg: "HTTP response is empty or not found.",
-        code: "CONJ00094E"
-      )
-
       MissingClaim = ::Util::TrackableErrorClass.new(
         msg: "Claim is empty or not found.",
         code: "CONJ00095E"
       )
 
-      InvalidHttpResponseFormat = ::Util::TrackableErrorClass.new(
-        msg: "HTTP response format is invalid",
-        code: "CONJ00096E"
-      )
-
       ServiceIdMissing = ::Util::TrackableErrorClass.new(
         msg: "Service ID is required when authenticating with authn-jwt",
         code: "CONJ00097E"
@@ -588,6 +578,11 @@ module AuthnJwt
         msg: "Invalid signing key settings: {0-validation-error}",
         code: "CONJ00122E"
       )
+
+      FailedToFetchJwksData = ::Util::TrackableErrorClass.new(
+        msg: "Failed to fetch JWKS data from '{0-jwks-uri}' with error: {1-error}",
+        code: "CONJ00123E"
+      )
     end
 
     module ResourceRestrictions

diff --git a/spec/app/domain/authentication/authn-jwt/signing_key/create_jwks_from_http_response_spec.rb b/spec/app/domain/authentication/authn-jwt/signing_key/create_jwks_from_http_response_spec.rb
@@ -4,66 +4,59 @@
 
 RSpec.describe('Authentication::AuthnJwt::SigningKey::CreateJwksFromHttpResponse') do
 
-  let(:mocked_http_response_without_body) { double("MockedHttpResponse") }
+  let(:mocked_http_response_unsuccessful) { double("MockedHttpResponse") }
+  let(:http_error) { "400 Bad Request" }
+  let(:http_url) { "https://jwks/address" }
   let(:mocked_http_response_with_invalid_json_structure) { double("MockedHttpResponse") }
   let(:mocked_http_response_without_keys) { double("MockedHttpResponse") }
   let(:mocked_http_response_with_empty_keys) { double("MockedHttpResponse") }
   let(:mocked_http_response_with_valid_keys) { double("MockedHttpResponse") }
-  let(:http_response_invalid_json_structure) { "{  invalid:  {  structure: true  }" }
-  let(:http_response_without_keys) { '{"no_keys":[{"kty":"RSA","kid":"kewiQq9jiC84CvSsJYOB-N6A8WFLSV20Mb-y7IlWDSQ","e":"AQAB","n":"5RyvCSgBoOGNE03CMcJ9Bzo1JDvsU8XgddvRuJtdJAIq5zJ8fiUEGCnMfAZI4of36YXBuBalIycqkgxrRkSOENRUCWN45bf8xsQCcQ8zZxozu0St4w5S-aC7N7UTTarPZTp4BZH8ttUm-VnK4aEdMx9L3Izo0hxaJ135undTuA6gQpK-0nVsm6tRVq4akDe3OhC-7b2h6z7GWJX1SD4sAD3iaq4LZa8y1mvBBz6AIM9co8R-vU1_CduxKQc3KxCnqKALbEKXm0mTGsXha9aNv3pLNRNs_J-cCjBpb1EXAe_7qOURTiIHdv8_sdjcFTJ0OTeLWywuSf7mD0Wpx2LKcD6ImENbyq5IBuR1e2ghnh5Y9H33cuQ0FRni8ikq5W3xP3HSMfwlayhIAJN_WnmbhENRU-m2_hDPiD9JYF2CrQneLkE3kcazSdtarPbg9ZDiydHbKWCV-X7HxxIKEr9N7P1V5HKatF4ZUrG60e3eBnRyccPwmT66i9NYyrcy1_ZNN8D1DY8xh9kflUDy4dSYu4R7AEWxNJWQQov525v0MjD5FNAS03rpk4SuW3Mt7IP73m-_BpmIhW3LZsnmfd8xHRjf0M9veyJD0--ETGmh8t3_CXh3I3R9IbcSEntUl_2lCvc_6B-m8W-t2nZr4wvOq9-iaTQXAn1Au6EaOYWvDRE","use":"sig","alg":"RS256"},{"kty":"RSA","kid":"4i3sFE7sxqNPOT7FdvcGA1ZVGGI_r-tsDXnEuYT4ZqE","e":"AQAB","n":"4cxDjTcJRJFID6UCgepPV45T1XDz_cLXSPgMur00WXB4jJrR9bfnZDx6dWqwps2dCw-lD3Fccj2oItwdRQ99In61l48MgiJaITf5JK2c63halNYiNo22_cyBG__nCkDZTZwEfGdfPRXSOWMg1E0pgGc1PoqwOdHZrQVqTcP3vWJt8bDQSOuoZBHSwVzDSjHPY6LmJMEO42H27t3ZkcYtS5crU8j2Yf-UH5U6rrSEyMdrCpc9IXe9WCmWjz5yOQa0r3U7M5OPEKD1-8wuP6_dPw0DyNO_Ei7UerVtsx5XSTd-Z5ujeB3PFVeAdtGxJ23oRNCq2MCOZBa58EGeRDLR7Q","use":"sig","alg":"RS256"}]}' }
-  let(:http_response_with_empty_keys) { '{"keys":[]}' }
-  let(:http_response_with_valid_keys) { '{"keys":[{"kty":"RSA","kid":"kewiQq9jiC84CvSsJYOB-N6A8WFLSV20Mb-y7IlWDSQ","e":"AQAB","n":"5RyvCSgBoOGNE03CMcJ9Bzo1JDvsU8XgddvRuJtdJAIq5zJ8fiUEGCnMfAZI4of36YXBuBalIycqkgxrRkSOENRUCWN45bf8xsQCcQ8zZxozu0St4w5S-aC7N7UTTarPZTp4BZH8ttUm-VnK4aEdMx9L3Izo0hxaJ135undTuA6gQpK-0nVsm6tRVq4akDe3OhC-7b2h6z7GWJX1SD4sAD3iaq4LZa8y1mvBBz6AIM9co8R-vU1_CduxKQc3KxCnqKALbEKXm0mTGsXha9aNv3pLNRNs_J-cCjBpb1EXAe_7qOURTiIHdv8_sdjcFTJ0OTeLWywuSf7mD0Wpx2LKcD6ImENbyq5IBuR1e2ghnh5Y9H33cuQ0FRni8ikq5W3xP3HSMfwlayhIAJN_WnmbhENRU-m2_hDPiD9JYF2CrQneLkE3kcazSdtarPbg9ZDiydHbKWCV-X7HxxIKEr9N7P1V5HKatF4ZUrG60e3eBnRyccPwmT66i9NYyrcy1_ZNN8D1DY8xh9kflUDy4dSYu4R7AEWxNJWQQov525v0MjD5FNAS03rpk4SuW3Mt7IP73m-_BpmIhW3LZsnmfd8xHRjf0M9veyJD0--ETGmh8t3_CXh3I3R9IbcSEntUl_2lCvc_6B-m8W-t2nZr4wvOq9-iaTQXAn1Au6EaOYWvDRE","use":"sig","alg":"RS256"},{"kty":"RSA","kid":"4i3sFE7sxqNPOT7FdvcGA1ZVGGI_r-tsDXnEuYT4ZqE","e":"AQAB","n":"4cxDjTcJRJFID6UCgepPV45T1XDz_cLXSPgMur00WXB4jJrR9bfnZDx6dWqwps2dCw-lD3Fccj2oItwdRQ99In61l48MgiJaITf5JK2c63halNYiNo22_cyBG__nCkDZTZwEfGdfPRXSOWMg1E0pgGc1PoqwOdHZrQVqTcP3vWJt8bDQSOuoZBHSwVzDSjHPY6LmJMEO42H27t3ZkcYtS5crU8j2Yf-UH5U6rrSEyMdrCpc9IXe9WCmWjz5yOQa0r3U7M5OPEKD1-8wuP6_dPw0DyNO_Ei7UerVtsx5XSTd-Z5ujeB3PFVeAdtGxJ23oRNCq2MCOZBa58EGeRDLR7Q","use":"sig","alg":"RS256"}]}' }
-  let(:valid_jwks) { {:keys => JSON::JWK::Set.new(JSON.parse(http_response_with_valid_keys)['keys'])} }
+  let(:http_body_invalid_json_structure) { "{  invalid:  {  structure: true  }" }
+  let(:http_body_without_keys) { '{"no_keys":[{"kty":"RSA","kid":"kewiQq9jiC84CvSsJYOB-N6A8WFLSV20Mb-y7IlWDSQ","e":"AQAB","n":"5RyvCSgBoOGNE03CMcJ9Bzo1JDvsU8XgddvRuJtdJAIq5zJ8fiUEGCnMfAZI4of36YXBuBalIycqkgxrRkSOENRUCWN45bf8xsQCcQ8zZxozu0St4w5S-aC7N7UTTarPZTp4BZH8ttUm-VnK4aEdMx9L3Izo0hxaJ135undTuA6gQpK-0nVsm6tRVq4akDe3OhC-7b2h6z7GWJX1SD4sAD3iaq4LZa8y1mvBBz6AIM9co8R-vU1_CduxKQc3KxCnqKALbEKXm0mTGsXha9aNv3pLNRNs_J-cCjBpb1EXAe_7qOURTiIHdv8_sdjcFTJ0OTeLWywuSf7mD0Wpx2LKcD6ImENbyq5IBuR1e2ghnh5Y9H33cuQ0FRni8ikq5W3xP3HSMfwlayhIAJN_WnmbhENRU-m2_hDPiD9JYF2CrQneLkE3kcazSdtarPbg9ZDiydHbKWCV-X7HxxIKEr9N7P1V5HKatF4ZUrG60e3eBnRyccPwmT66i9NYyrcy1_ZNN8D1DY8xh9kflUDy4dSYu4R7AEWxNJWQQov525v0MjD5FNAS03rpk4SuW3Mt7IP73m-_BpmIhW3LZsnmfd8xHRjf0M9veyJD0--ETGmh8t3_CXh3I3R9IbcSEntUl_2lCvc_6B-m8W-t2nZr4wvOq9-iaTQXAn1Au6EaOYWvDRE","use":"sig","alg":"RS256"},{"kty":"RSA","kid":"4i3sFE7sxqNPOT7FdvcGA1ZVGGI_r-tsDXnEuYT4ZqE","e":"AQAB","n":"4cxDjTcJRJFID6UCgepPV45T1XDz_cLXSPgMur00WXB4jJrR9bfnZDx6dWqwps2dCw-lD3Fccj2oItwdRQ99In61l48MgiJaITf5JK2c63halNYiNo22_cyBG__nCkDZTZwEfGdfPRXSOWMg1E0pgGc1PoqwOdHZrQVqTcP3vWJt8bDQSOuoZBHSwVzDSjHPY6LmJMEO42H27t3ZkcYtS5crU8j2Yf-UH5U6rrSEyMdrCpc9IXe9WCmWjz5yOQa0r3U7M5OPEKD1-8wuP6_dPw0DyNO_Ei7UerVtsx5XSTd-Z5ujeB3PFVeAdtGxJ23oRNCq2MCOZBa58EGeRDLR7Q","use":"sig","alg":"RS256"}]}' }
+  let(:http_body_with_empty_keys) { '{"keys":[]}' }
+  let(:http_body_with_valid_keys) { '{"keys":[{"kty":"RSA","kid":"kewiQq9jiC84CvSsJYOB-N6A8WFLSV20Mb-y7IlWDSQ","e":"AQAB","n":"5RyvCSgBoOGNE03CMcJ9Bzo1JDvsU8XgddvRuJtdJAIq5zJ8fiUEGCnMfAZI4of36YXBuBalIycqkgxrRkSOENRUCWN45bf8xsQCcQ8zZxozu0St4w5S-aC7N7UTTarPZTp4BZH8ttUm-VnK4aEdMx9L3Izo0hxaJ135undTuA6gQpK-0nVsm6tRVq4akDe3OhC-7b2h6z7GWJX1SD4sAD3iaq4LZa8y1mvBBz6AIM9co8R-vU1_CduxKQc3KxCnqKALbEKXm0mTGsXha9aNv3pLNRNs_J-cCjBpb1EXAe_7qOURTiIHdv8_sdjcFTJ0OTeLWywuSf7mD0Wpx2LKcD6ImENbyq5IBuR1e2ghnh5Y9H33cuQ0FRni8ikq5W3xP3HSMfwlayhIAJN_WnmbhENRU-m2_hDPiD9JYF2CrQneLkE3kcazSdtarPbg9ZDiydHbKWCV-X7HxxIKEr9N7P1V5HKatF4ZUrG60e3eBnRyccPwmT66i9NYyrcy1_ZNN8D1DY8xh9kflUDy4dSYu4R7AEWxNJWQQov525v0MjD5FNAS03rpk4SuW3Mt7IP73m-_BpmIhW3LZsnmfd8xHRjf0M9veyJD0--ETGmh8t3_CXh3I3R9IbcSEntUl_2lCvc_6B-m8W-t2nZr4wvOq9-iaTQXAn1Au6EaOYWvDRE","use":"sig","alg":"RS256"},{"kty":"RSA","kid":"4i3sFE7sxqNPOT7FdvcGA1ZVGGI_r-tsDXnEuYT4ZqE","e":"AQAB","n":"4cxDjTcJRJFID6UCgepPV45T1XDz_cLXSPgMur00WXB4jJrR9bfnZDx6dWqwps2dCw-lD3Fccj2oItwdRQ99In61l48MgiJaITf5JK2c63halNYiNo22_cyBG__nCkDZTZwEfGdfPRXSOWMg1E0pgGc1PoqwOdHZrQVqTcP3vWJt8bDQSOuoZBHSwVzDSjHPY6LmJMEO42H27t3ZkcYtS5crU8j2Yf-UH5U6rrSEyMdrCpc9IXe9WCmWjz5yOQa0r3U7M5OPEKD1-8wuP6_dPw0DyNO_Ei7UerVtsx5XSTd-Z5ujeB3PFVeAdtGxJ23oRNCq2MCOZBa58EGeRDLR7Q","use":"sig","alg":"RS256"}]}' }
+  let(:valid_jwks) { {:keys => JSON::JWK::Set.new(JSON.parse(http_body_with_valid_keys)['keys'])} }
 
   before(:each) do
-    allow(mocked_http_response_with_invalid_json_structure).to(
-      receive(:blank?).and_return(false)
+    allow(mocked_http_response_unsuccessful).to(
+      receive(:value).and_raise(http_error)
     )
 
-    allow(mocked_http_response_with_invalid_json_structure).to(
-      receive(:respond_to?).with(:body).and_return(true)
+    allow(mocked_http_response_unsuccessful).to(
+      receive(:uri).and_return(http_url)
     )
 
     allow(mocked_http_response_with_invalid_json_structure).to(
-      receive(:body).and_return(http_response_invalid_json_structure)
+      receive(:value)
     )
 
-    allow(mocked_http_response_without_keys).to(
-      receive(:blank?).and_return(false)
+    allow(mocked_http_response_with_invalid_json_structure).to(
+      receive(:body).and_return(http_body_invalid_json_structure)
     )
 
     allow(mocked_http_response_without_keys).to(
-      receive(:respond_to?).with(:body).and_return(true)
+      receive(:value)
     )
 
     allow(mocked_http_response_without_keys).to(
-      receive(:body).and_return(http_response_without_keys)
-    )
-
-    allow(mocked_http_response_with_empty_keys).to(
-      receive(:blank?).and_return(false)
+      receive(:body).and_return(http_body_without_keys)
     )
 
     allow(mocked_http_response_with_empty_keys).to(
-      receive(:respond_to?).with(:body).and_return(true)
+      receive(:value)
     )
 
     allow(mocked_http_response_with_empty_keys).to(
-      receive(:body).and_return(http_response_with_empty_keys)
+      receive(:body).and_return(http_body_with_empty_keys)
     )
 
     allow(mocked_http_response_with_valid_keys).to(
-      receive(:blank?).and_return(false)
+      receive(:value)
     )
 
     allow(mocked_http_response_with_valid_keys).to(
-      receive(:respond_to?).with(:body).and_return(true)
+      receive(:body).and_return(http_body_with_valid_keys)
     )
-
-    allow(mocked_http_response_with_valid_keys).to(
-      receive(:body).and_return(http_response_with_valid_keys)
-    )
-
   end
 
   #  ____  _   _  ____    ____  ____  ___  ____  ___
@@ -72,45 +65,23 @@
   #  (__) (_) (_)(____)   (__) (____)(___/ (__) (___/
 
   context "'http_response' input" do
-    context "with nil value" do
-      subject do
-        ::Authentication::AuthnJwt::SigningKey::CreateJwksFromHttpResponse.new().call(
-          http_response: nil
-        )
-      end
-
-      it "raises an error" do
-        expect { subject }.to raise_error(Errors::Authentication::AuthnJwt::MissingHttpResponse)
-      end
-    end
-
-    context "with empty value" do
-      subject do
-        ::Authentication::AuthnJwt::SigningKey::CreateJwksFromHttpResponse.new().call(
-          http_response: ""
-        )
-      end
-
-      it "raises an error" do
-        expect { subject }.to raise_error(Errors::Authentication::AuthnJwt::MissingHttpResponse)
-      end
-    end
-
-    context "without body" do
+    context "with unsuccessful http response" do
       subject do
-        ::Authentication::AuthnJwt::SigningKey::CreateJwksFromHttpResponse.new().call(
-          http_response: mocked_http_response_without_body
+        ::Authentication::AuthnJwt::SigningKey::CreateJwksFromHttpResponse.new.call(
+          http_response: mocked_http_response_unsuccessful
         )
       end
 
       it "raises an error" do
-        expect { subject }.to raise_error(Errors::Authentication::AuthnJwt::InvalidHttpResponseFormat)
+        expect { subject }.to raise_error(
+                                Errors::Authentication::AuthnJwt::FailedToFetchJwksData,
+                                /.*'#{http_url}' with error: #<RuntimeError: #{http_error}.*/)
       end
     end
 
     context "with invalid json structure" do
       subject do
-        ::Authentication::AuthnJwt::SigningKey::CreateJwksFromHttpResponse.new().call(
+        ::Authentication::AuthnJwt::SigningKey::CreateJwksFromHttpResponse.new.call(
           http_response: mocked_http_response_with_invalid_json_structure
         )
       end
@@ -123,7 +94,7 @@
     context "with valid json structure" do
       context "when 'keys' are missing" do
         subject do
-          ::Authentication::AuthnJwt::SigningKey::CreateJwksFromHttpResponse.new().call(
+          ::Authentication::AuthnJwt::SigningKey::CreateJwksFromHttpResponse.new.call(
             http_response: mocked_http_response_without_keys
           )
         end
@@ -135,7 +106,7 @@
 
       context "with empty 'keys' value" do
         subject do
-          ::Authentication::AuthnJwt::SigningKey::CreateJwksFromHttpResponse.new().call(
+          ::Authentication::AuthnJwt::SigningKey::CreateJwksFromHttpResponse.new.call(
             http_response: mocked_http_response_with_empty_keys
           )
         end
@@ -147,7 +118,7 @@
 
       context "with valid 'keys' value" do
         subject do
-          ::Authentication::AuthnJwt::SigningKey::CreateJwksFromHttpResponse.new().call(
+          ::Authentication::AuthnJwt::SigningKey::CreateJwksFromHttpResponse.new.call(
             http_response: mocked_http_response_with_valid_keys
           )
         end