From 20dc5c9ed5762b2f42dfe6e267571f09a6af3c8c Mon Sep 17 00:00:00 2001 From: Sasha Chernomordik Date: Sun, 19 Dec 2021 18:06:38 +0200 Subject: [PATCH 1/2] Signing key fetchers are using cache instead of upper level classes --- .../create_signing_key_provider.rb | 16 ++- .../signing_key/fetch_jwks_uri_signing_key.rb | 14 +- .../fetch_provider_uri_signing_key.rb | 14 +- .../validate_and_decode_token.rb | 16 +-- .../authn_jwt/validate_status.rb | 15 +- .../fetch_jwks_signing_key_spec.rb | 46 +++++- .../fetch_provider_uri_signing_key_spec.rb | 47 ++++++- .../validate_and_decode_token_spec.rb | 133 ++++++------------ .../authn-jwt/validate_status_spec.rb | 22 +-- 9 files changed, 169 insertions(+), 154 deletions(-) diff --git a/app/domain/authentication/authn_jwt/signing_key/create_signing_key_provider.rb b/app/domain/authentication/authn_jwt/signing_key/create_signing_key_provider.rb index b309590b2b..792d70bd4a 100644 --- a/app/domain/authentication/authn_jwt/signing_key/create_signing_key_provider.rb +++ b/app/domain/authentication/authn_jwt/signing_key/create_signing_key_provider.rb @@ -4,6 +4,16 @@ module SigningKey # Factory that returns the interface implementation of FetchSigningKey CreateSigningKeyProvider ||= CommandClass.new( dependencies: { + fetch_signing_key: ::Util::ConcurrencyLimitedCache.new( + ::Util::RateLimitedCache.new( + ::Authentication::AuthnJwt::SigningKey::FetchCachedSigningKey.new, + refreshes_per_interval: CACHE_REFRESHES_PER_INTERVAL, + rate_limit_interval: CACHE_RATE_LIMIT_INTERVAL, + logger: Rails.logger + ), + max_concurrent_requests: CACHE_MAX_CONCURRENT_REQUESTS, + logger: Rails.logger + ), fetch_provider_uri_signing_key_class: Authentication::AuthnJwt::SigningKey::FetchProviderUriSigningKey, fetch_jwks_uri_signing_key_class: Authentication::AuthnJwt::SigningKey::FetchJwksUriSigningKey, check_authenticator_secret_exists: Authentication::Util::CheckAuthenticatorSecretExists.new, @@ -48,7 +58,8 @@ def fetch_provider_uri_signing_key LogMessages::Authentication::AuthnJwt::SelectedSigningKeyInterface.new(PROVIDER_URI_INTERFACE_NAME) ) @fetch_provider_uri_signing_key ||= @fetch_provider_uri_signing_key_class.new( - authenticator_input: @authenticator_input + authenticator_input: @authenticator_input, + fetch_signing_key: @fetch_signing_key ) end @@ -69,7 +80,8 @@ def fetch_jwks_uri_signing_key LogMessages::Authentication::AuthnJwt::SelectedSigningKeyInterface.new(JWKS_URI_INTERFACE_NAME) ) @fetch_jwks_uri_signing_key ||= @fetch_jwks_uri_signing_key_class.new( - authenticator_input: @authenticator_input + authenticator_input: @authenticator_input, + fetch_signing_key: @fetch_signing_key ) end end diff --git a/app/domain/authentication/authn_jwt/signing_key/fetch_jwks_uri_signing_key.rb b/app/domain/authentication/authn_jwt/signing_key/fetch_jwks_uri_signing_key.rb index cd65933ecb..4d7772b2b8 100644 --- a/app/domain/authentication/authn_jwt/signing_key/fetch_jwks_uri_signing_key.rb +++ b/app/domain/authentication/authn_jwt/signing_key/fetch_jwks_uri_signing_key.rb @@ -10,6 +10,7 @@ class FetchJwksUriSigningKey def initialize( authenticator_input:, + fetch_signing_key:, fetch_authenticator_secrets: Authentication::Util::FetchAuthenticatorSecrets.new, http_lib: Net::HTTP, create_jwks_from_http_response: CreateJwksFromHttpResponse.new, @@ -21,6 +22,15 @@ def initialize( @fetch_authenticator_secrets = fetch_authenticator_secrets @authenticator_input = authenticator_input + @fetch_signing_key = fetch_signing_key + end + + def call(force_read:) + @fetch_signing_key.call( + refresh: force_read, + cache_key: jwks_uri, + signing_key_provider: self + ) end def fetch_signing_key @@ -28,10 +38,6 @@ def fetch_signing_key fetch_jwks_keys end - def signing_key_uri - jwks_uri - end - private def fetch_jwks_uri diff --git a/app/domain/authentication/authn_jwt/signing_key/fetch_provider_uri_signing_key.rb b/app/domain/authentication/authn_jwt/signing_key/fetch_provider_uri_signing_key.rb index 6af6f8a4f1..795a6ea5a8 100644 --- a/app/domain/authentication/authn_jwt/signing_key/fetch_provider_uri_signing_key.rb +++ b/app/domain/authentication/authn_jwt/signing_key/fetch_provider_uri_signing_key.rb @@ -6,6 +6,7 @@ class FetchProviderUriSigningKey def initialize( authenticator_input:, + fetch_signing_key:, fetch_authenticator_secrets: Authentication::Util::FetchAuthenticatorSecrets.new, discover_identity_provider: Authentication::OAuth::DiscoverIdentityProvider.new, logger: Rails.logger @@ -15,6 +16,15 @@ def initialize( @discover_identity_provider = discover_identity_provider @authenticator_input = authenticator_input + @fetch_signing_key = fetch_signing_key + end + + def call(force_read:) + @fetch_signing_key.call( + refresh: force_read, + cache_key: provider_uri, + signing_key_provider: self + ) end def fetch_signing_key @@ -22,10 +32,6 @@ def fetch_signing_key fetch_provider_keys end - def signing_key_uri - provider_uri - end - private def discover_provider diff --git a/app/domain/authentication/authn_jwt/validate_and_decode/validate_and_decode_token.rb b/app/domain/authentication/authn_jwt/validate_and_decode/validate_and_decode_token.rb index 91c549c46b..fba7169ad4 100644 --- a/app/domain/authentication/authn_jwt/validate_and_decode/validate_and_decode_token.rb +++ b/app/domain/authentication/authn_jwt/validate_and_decode/validate_and_decode_token.rb @@ -7,16 +7,6 @@ module ValidateAndDecode # for the 2nd validation ValidateAndDecodeToken ||= CommandClass.new( dependencies: { - fetch_signing_key: ::Util::ConcurrencyLimitedCache.new( - ::Util::RateLimitedCache.new( - ::Authentication::AuthnJwt::SigningKey::FetchCachedSigningKey.new, - refreshes_per_interval: CACHE_REFRESHES_PER_INTERVAL, - rate_limit_interval: CACHE_RATE_LIMIT_INTERVAL, - logger: Rails.logger - ), - max_concurrent_requests: CACHE_MAX_CONCURRENT_REQUESTS, - logger: Rails.logger - ), verify_and_decode_token: ::Authentication::Jwt::VerifyAndDecodeToken.new, fetch_jwt_claims_to_validate: ::Authentication::AuthnJwt::ValidateAndDecode::FetchJwtClaimsToValidate.new, get_verification_option_by_jwt_claim: ::Authentication::AuthnJwt::ValidateAndDecode::GetVerificationOptionByJwtClaim.new, @@ -52,10 +42,8 @@ def validate_token_exists end def fetch_signing_key(force_read: false) - @jwks = @fetch_signing_key.call( - refresh: force_read, - cache_key: signing_key_provider.signing_key_uri, - signing_key_provider: signing_key_provider + @jwks = signing_key_provider.call( + force_read: force_read ) @logger.debug(LogMessages::Authentication::AuthnJwt::SigningKeysFetchedFromCache.new) end diff --git a/app/domain/authentication/authn_jwt/validate_status.rb b/app/domain/authentication/authn_jwt/validate_status.rb index a27358e568..097dbb5b63 100644 --- a/app/domain/authentication/authn_jwt/validate_status.rb +++ b/app/domain/authentication/authn_jwt/validate_status.rb @@ -3,16 +3,6 @@ module AuthnJwt ValidateStatus = CommandClass.new( dependencies: { - fetch_signing_key: ::Util::ConcurrencyLimitedCache.new( - ::Util::RateLimitedCache.new( - ::Authentication::AuthnJwt::SigningKey::FetchCachedSigningKey.new, - refreshes_per_interval: CACHE_REFRESHES_PER_INTERVAL, - rate_limit_interval: CACHE_RATE_LIMIT_INTERVAL, - logger: Rails.logger - ), - max_concurrent_requests: CACHE_MAX_CONCURRENT_REQUESTS, - logger: Rails.logger - ), create_signing_key_provider: Authentication::AuthnJwt::SigningKey::CreateSigningKeyProvider.new, fetch_issuer_value: Authentication::AuthnJwt::ValidateAndDecode::FetchIssuerValue.new, fetch_audience_value: Authentication::AuthnJwt::ValidateAndDecode::FetchAudienceValue.new, @@ -150,9 +140,8 @@ def webservice end def validate_signing_key - @fetch_signing_key.call( - cache_key: signing_key_provider.signing_key_uri, - signing_key_provider: signing_key_provider + signing_key_provider.call( + force_read: false ) @logger.debug(LogMessages::Authentication::AuthnJwt::ValidatedSigningKeyConfiguration.new) end diff --git a/spec/app/domain/authentication/authn-jwt/signing_key/fetch_jwks_signing_key_spec.rb b/spec/app/domain/authentication/authn-jwt/signing_key/fetch_jwks_signing_key_spec.rb index 06d14d8dde..0f7116638b 100644 --- a/spec/app/domain/authentication/authn-jwt/signing_key/fetch_jwks_signing_key_spec.rb +++ b/spec/app/domain/authentication/authn-jwt/signing_key/fetch_jwks_signing_key_spec.rb @@ -24,6 +24,8 @@ let(:bad_response_error) { "bad response error" } let(:required_secret_missing_error) { "required secret missing error" } let(:mocked_logger) { double("Mocked Logger") } + let(:mocked_fetch_signing_key) { double("MockedFetchSigningKey") } + let(:mocked_fetch_signing_key_refresh_value) { double("MockedFetchSigningKeyRefreshValue") } let(:mocked_fetch_authenticator_secrets_exist_values) { double("MockedFetchAuthenticatorSecrets") } let(:mocked_fetch_authenticator_secrets_empty_values) { double("MockedFetchAuthenticatorSecrets") } let(:mocked_bad_http_response) { double("Mocked bad http response") } @@ -49,6 +51,9 @@ receive(:info).and_return(true) ) + allow(mocked_fetch_signing_key).to receive(:call) { |params| params[:signing_key_provider].fetch_signing_key } + allow(mocked_fetch_signing_key_refresh_value).to receive(:call) { |params| params[:refresh] } + allow(mocked_fetch_authenticator_secrets_exist_values).to( receive(:call).and_return('jwks-uri' => 'https://jwks-uri.com/jwks') ) @@ -79,17 +84,48 @@ # )( ) _ ( )__) )( )__) \__ \ )( \__ \ # (__) (_) (_)(____) (__) (____)(___/ (__) (___/ + context "FetchJwksUriSigningKey call " do + context "propagates false refresh value" do + subject do + ::Authentication::AuthnJwt::SigningKey::FetchJwksUriSigningKey.new(authenticator_input: mocked_authenticator_input, + fetch_signing_key: mocked_fetch_signing_key_refresh_value, + logger: mocked_logger, + fetch_authenticator_secrets: mocked_fetch_authenticator_secrets_exist_values, + http_lib: mocked_bad_http_response, + create_jwks_from_http_response: mocked_create_jwks_from_http_response + ).call(force_read: false) + end + + it "returns false" do + expect(subject).to eql(false) + end + end + context "propagates true refresh value" do + subject do + ::Authentication::AuthnJwt::SigningKey::FetchJwksUriSigningKey.new(authenticator_input: mocked_authenticator_input, + fetch_signing_key: mocked_fetch_signing_key_refresh_value, + logger: mocked_logger, + fetch_authenticator_secrets: mocked_fetch_authenticator_secrets_exist_values, + http_lib: mocked_bad_http_response, + create_jwks_from_http_response: mocked_create_jwks_from_http_response + ).call(force_read: true) + end + + it "returns true" do + expect(subject).to eql(true) + end + end - context "FetchJwksUriSigningKey fetch_signing_key " do context "'jwks-uri' secret is not valid" do subject do ::Authentication::AuthnJwt::SigningKey::FetchJwksUriSigningKey.new(authenticator_input: mocked_authenticator_input, + fetch_signing_key: mocked_fetch_signing_key, logger: mocked_logger, fetch_authenticator_secrets: mocked_fetch_authenticator_secrets_exist_values, http_lib: mocked_bad_http_response, create_jwks_from_http_response: mocked_create_jwks_from_http_response - ).fetch_signing_key + ).call(force_read: false) end it "raises an error" do @@ -101,11 +137,12 @@ context "provider return valid http response" do subject do ::Authentication::AuthnJwt::SigningKey::FetchJwksUriSigningKey.new(authenticator_input: mocked_authenticator_input, + fetch_signing_key: mocked_fetch_signing_key, logger: mocked_logger, fetch_authenticator_secrets: mocked_fetch_authenticator_secrets_exist_values, http_lib: mocked_good_http_response, create_jwks_from_http_response: mocked_create_jwks_from_http_response - ).fetch_signing_key + ).call(force_read: false) end it "returns jwks value" do @@ -116,11 +153,12 @@ context "provider return bad http response" do subject do ::Authentication::AuthnJwt::SigningKey::FetchJwksUriSigningKey.new(authenticator_input: mocked_authenticator_input, + fetch_signing_key: mocked_fetch_signing_key, logger: mocked_logger, fetch_authenticator_secrets: mocked_fetch_authenticator_secrets_exist_values, http_lib: mocked_bad_http_response, create_jwks_from_http_response: mocked_create_jwks_from_http_response - ).fetch_signing_key + ).call(force_read: false) end it "raises an error" do diff --git a/spec/app/domain/authentication/authn-jwt/signing_key/fetch_provider_uri_signing_key_spec.rb b/spec/app/domain/authentication/authn-jwt/signing_key/fetch_provider_uri_signing_key_spec.rb index b70a55bc9b..37332814d2 100644 --- a/spec/app/domain/authentication/authn-jwt/signing_key/fetch_provider_uri_signing_key_spec.rb +++ b/spec/app/domain/authentication/authn-jwt/signing_key/fetch_provider_uri_signing_key_spec.rb @@ -24,6 +24,8 @@ let(:required_secret_missing_error) { "required secret missing error" } let(:mocked_logger) { double("Mocked Logger") } + let(:mocked_fetch_signing_key) { double("MockedFetchSigningKey") } + let(:mocked_fetch_signing_key_refresh_value) { double("MockedFetchSigningKeyRefreshValue") } let(:mocked_fetch_authenticator_secrets_exist_values) { double("MockedFetchAuthenticatorSecrets") } let(:mocked_fetch_authenticator_secrets_empty_values) { double("MockedFetchAuthenticatorSecrets") } let(:mocked_discover_identity_provider) { double("Mocked discover identity provider") } @@ -46,6 +48,9 @@ receive(:info).and_return(true) ) + allow(mocked_fetch_signing_key).to receive(:call) { |params| params[:signing_key_provider].fetch_signing_key } + allow(mocked_fetch_signing_key_refresh_value).to receive(:call) { |params| params[:refresh] } + allow(mocked_fetch_authenticator_secrets_exist_values).to( receive(:call).and_return('provider-uri' => 'https://provider-uri.com/provider') ) @@ -72,14 +77,48 @@ # )( ) _ ( )__) )( )__) \__ \ )( \__ \ # (__) (_) (_)(____) (__) (____)(___/ (__) (___/ - context "FetchProviderUriSigningKey fetch_signing_key " do + context "FetchProviderUriSigningKey call " do + context "propagates refresh value" do + context "false" do + subject do + ::Authentication::AuthnJwt::SigningKey::FetchProviderUriSigningKey.new(authenticator_input: mocked_authenticator_input, + fetch_signing_key: mocked_fetch_signing_key_refresh_value, + logger: mocked_logger, + fetch_authenticator_secrets: mocked_fetch_authenticator_secrets_exist_values, + discover_identity_provider: mocked_discover_identity_provider + ).call(force_read: false) + end + + it "returns false" do + expect(subject).to eql(false) + end + end + + context "true" do + subject do + ::Authentication::AuthnJwt::SigningKey::FetchProviderUriSigningKey.new(authenticator_input: mocked_authenticator_input, + fetch_signing_key: mocked_fetch_signing_key_refresh_value, + logger: mocked_logger, + fetch_authenticator_secrets: mocked_fetch_authenticator_secrets_exist_values, + discover_identity_provider: mocked_discover_identity_provider + ).call(force_read: true) + end + + it "returns true" do + expect(subject).to eql(true) + end + end + end + context "'provider-uri' variable is configured in authenticator policy" do context "'provider-uri' value is invalid" do subject do ::Authentication::AuthnJwt::SigningKey::FetchProviderUriSigningKey.new(authenticator_input: mocked_authenticator_input, + fetch_signing_key: mocked_fetch_signing_key, logger: mocked_logger, fetch_authenticator_secrets: mocked_fetch_authenticator_secrets_exist_values, - discover_identity_provider: mocked_invalid_uri_discover_identity_provider).fetch_signing_key + discover_identity_provider: mocked_invalid_uri_discover_identity_provider + ).call(force_read: false) end it "raises an error" do @@ -90,9 +129,11 @@ context "'provider-uri' value is valid" do subject do ::Authentication::AuthnJwt::SigningKey::FetchProviderUriSigningKey.new(authenticator_input: mocked_authenticator_input, + fetch_signing_key: mocked_fetch_signing_key, logger: mocked_logger, fetch_authenticator_secrets: mocked_fetch_authenticator_secrets_exist_values, - discover_identity_provider: mocked_discover_identity_provider).fetch_signing_key + discover_identity_provider: mocked_discover_identity_provider + ).call(force_read: false) end it "does not raise error" do diff --git a/spec/app/domain/authentication/authn-jwt/validate_and_decode/validate_and_decode_token_spec.rb b/spec/app/domain/authentication/authn-jwt/validate_and_decode/validate_and_decode_token_spec.rb index df1074de5c..4acc535f5b 100644 --- a/spec/app/domain/authentication/authn-jwt/validate_and_decode/validate_and_decode_token_spec.rb +++ b/spec/app/domain/authentication/authn-jwt/validate_and_decode/validate_and_decode_token_spec.rb @@ -17,20 +17,16 @@ ) } - let(:mocked_create_signing_key_provider_valid) { double("MockedSigningKeyInterfaceFactoryValid") } - let(:mocked_create_signing_key_provider_invalid) { double("MockedSigningKeyInterfaceFactoryInvalid") } let(:mocked_create_signing_key_provider_failed) { double("MockedSigningKeyInterfaceFactoryFailed") } + let(:mocked_create_signing_key_provider_always_succeed) { double("MockedSigningKeyInterfaceFactoryAlwaysSucceed") } + let(:mocked_create_signing_key_provider_failed_on_1st_time) { double("MockedSigningKeyInterfaceFactoryFailedOn1") } + let(:mocked_create_signing_key_provider_failed_on_2st_time) { double("MockedSigningKeyInterfaceFactoryFailedOn2") } let(:create_signing_key_provider_error) { "signing key interface factory error" } - let(:mocked_fetch_signing_key_provider_valid) { double("MockedSigningKeyInterfaceValid") } - let(:mocked_fetch_signing_key_provider_failed) { double("MockedSigningKeyInterfaceFailed") } - - let(:fetch_signing_key_provider_error) { "fetch signing key interface error" } - - let(:mocked_fetch_signing_key_failed_on_1st_time) { double("MockedFetchSigningKeyInvalid") } - let(:mocked_fetch_signing_key_failed_on_2nd_time) { double("MockedFetchSigningKeyInvalid") } - let(:mocked_fetch_signing_key_always_succeed) { double("MockedFetchSigningKey") } + let(:mocked_fetch_signing_key_provider_always_succeed) { double("MockedFetchSigningKeyProviderAlwaysSucceed") } + let(:mocked_fetch_signing_key_provider_failed_on_1st_time) { double("MockedFetchSigningKeyProviderFailedOn1") } + let(:mocked_fetch_signing_key_provider_failed_on_2nd_time) { double("MockedFetchSigningKeyProviderFailedOn2") } let(:fetch_signing_key_1st_time_error) { "fetch signing key 1st time error" } let(:fetch_signing_key_2nd_time_error) { "fetch signing key 2nd time error" } @@ -108,47 +104,49 @@ def valid_decoded_token(claims) let(:mocked_verify_and_decode_token_succeed_to_validate_claims_when_keys_updated) { double("MockedVerifyAndDecodeTokenSucceedToValidateClaims") } before(:each) do - allow(mocked_fetch_signing_key_provider_valid).to( - receive(:signing_key_uri).and_return(valid_signing_key_uri) + allow(mocked_create_signing_key_provider_failed).to( + receive(:call).and_raise(create_signing_key_provider_error) ) - allow(mocked_create_signing_key_provider_valid).to( - receive(:call).and_return(mocked_fetch_signing_key_provider_valid) + allow(mocked_create_signing_key_provider_always_succeed).to( + receive(:call).and_return(mocked_fetch_signing_key_provider_always_succeed) ) - allow(mocked_fetch_signing_key_provider_failed).to( - receive(:signing_key_uri).and_raise(fetch_signing_key_provider_error) + allow(mocked_create_signing_key_provider_failed_on_1st_time).to( + receive(:call).and_return(mocked_fetch_signing_key_provider_failed_on_1st_time) ) - allow(mocked_create_signing_key_provider_invalid).to( - receive(:call).and_return(mocked_fetch_signing_key_provider_failed) + allow(mocked_create_signing_key_provider_failed_on_2st_time).to( + receive(:call).and_return(mocked_fetch_signing_key_provider_failed_on_2nd_time) ) - allow(mocked_create_signing_key_provider_failed).to( - receive(:call).and_raise(create_signing_key_provider_error) + allow(mocked_fetch_signing_key_provider_always_succeed).to( + receive(:call).with( + force_read: false + ).and_return(jwks_from_1st_call) ) - allow(mocked_fetch_signing_key_failed_on_1st_time).to( + allow(mocked_fetch_signing_key_provider_always_succeed).to( receive(:call).with( - refresh: false, - cache_key: anything(), - signing_key_provider: anything() + force_read: true + ).and_return(jwks_from_2nd_call) + ) + + allow(mocked_fetch_signing_key_provider_failed_on_1st_time).to( + receive(:call).with( + force_read: false ).and_raise(fetch_signing_key_1st_time_error) ) - allow(mocked_fetch_signing_key_failed_on_2nd_time).to( + allow(mocked_fetch_signing_key_provider_failed_on_2nd_time).to( receive(:call).with( - refresh: false, - cache_key: anything(), - signing_key_provider: anything() + force_read: false ).and_return(jwks_from_2nd_call) ) - allow(mocked_fetch_signing_key_failed_on_2nd_time).to( + allow(mocked_fetch_signing_key_provider_failed_on_2nd_time).to( receive(:call).with( - refresh: true, - cache_key: anything(), - signing_key_provider: anything() + force_read: true ).and_raise(fetch_signing_key_2nd_time_error) ) @@ -156,22 +154,6 @@ def valid_decoded_token(claims) receive(:call).and_raise(verify_and_decode_token_error) ) - allow(mocked_fetch_signing_key_always_succeed).to( - receive(:call).with( - refresh: false, - cache_key: anything(), - signing_key_provider: anything() - ).and_return(jwks_from_1st_call) - ) - - allow(mocked_fetch_signing_key_always_succeed).to( - receive(:call).with( - refresh: true, - cache_key: anything(), - signing_key_provider: anything() - ).and_return(jwks_from_2nd_call) - ) - allow(mocked_verify_and_decode_token_succeed_on_1st_time).to( receive(:call).with( token_jwt: jwt_token_valid, @@ -319,7 +301,6 @@ def valid_decoded_token(claims) context "When error is during signing key factory call" do subject do ::Authentication::AuthnJwt::ValidateAndDecode::ValidateAndDecodeToken.new( - fetch_signing_key: mocked_fetch_signing_key_failed_on_1st_time, create_signing_key_provider: mocked_create_signing_key_provider_failed ).call( authenticator_input: authenticator_input, @@ -332,27 +313,10 @@ def valid_decoded_token(claims) end end - context "When error is during signing_key_uri call" do - subject do - ::Authentication::AuthnJwt::ValidateAndDecode::ValidateAndDecodeToken.new( - fetch_signing_key: mocked_fetch_signing_key_failed_on_1st_time, - create_signing_key_provider: mocked_create_signing_key_provider_invalid - ).call( - authenticator_input: authenticator_input, - jwt_token: jwt_token_valid - ) - end - - it "raises an error" do - expect { subject }.to raise_error(fetch_signing_key_provider_error) - end - end - - context "When error is during fetching from cache" do + context "When error is during fetching from provider" do subject do ::Authentication::AuthnJwt::ValidateAndDecode::ValidateAndDecodeToken.new( - fetch_signing_key: mocked_fetch_signing_key_failed_on_1st_time, - create_signing_key_provider: mocked_create_signing_key_provider_valid + create_signing_key_provider: mocked_create_signing_key_provider_failed_on_1st_time ).call( authenticator_input: authenticator_input, jwt_token: jwt_token_valid @@ -370,9 +334,8 @@ def valid_decoded_token(claims) context "and failed to fetch keys from provider" do subject do ::Authentication::AuthnJwt::ValidateAndDecode::ValidateAndDecodeToken.new( - fetch_signing_key: mocked_fetch_signing_key_failed_on_2nd_time, verify_and_decode_token: mocked_verify_and_decode_token_invalid, - create_signing_key_provider: mocked_create_signing_key_provider_valid + create_signing_key_provider: mocked_create_signing_key_provider_failed_on_2st_time ).call( authenticator_input: authenticator_input, jwt_token: jwt_token_valid @@ -387,9 +350,8 @@ def valid_decoded_token(claims) context "and succeed to fetch keys from provider" do subject do ::Authentication::AuthnJwt::ValidateAndDecode::ValidateAndDecodeToken.new( - fetch_signing_key: mocked_fetch_signing_key_always_succeed, verify_and_decode_token: mocked_verify_and_decode_token_invalid, - create_signing_key_provider: mocked_create_signing_key_provider_valid + create_signing_key_provider: mocked_create_signing_key_provider_always_succeed ).call( authenticator_input: authenticator_input, jwt_token: jwt_token_valid @@ -406,11 +368,10 @@ def valid_decoded_token(claims) context "and keys are not updated" do subject do ::Authentication::AuthnJwt::ValidateAndDecode::ValidateAndDecodeToken.new( - fetch_signing_key: mocked_fetch_signing_key_always_succeed, verify_and_decode_token: mocked_verify_and_decode_token_succeed_on_2nd_time, fetch_jwt_claims_to_validate: mocked_fetch_jwt_claims_to_validate_valid, get_verification_option_by_jwt_claim: mocked_get_verification_option_by_jwt_claim_valid, - create_signing_key_provider: mocked_create_signing_key_provider_valid + create_signing_key_provider: mocked_create_signing_key_provider_always_succeed ).call( authenticator_input: authenticator_input, jwt_token: jwt_token_valid @@ -425,11 +386,10 @@ def valid_decoded_token(claims) context "and keys are updated" do subject do ::Authentication::AuthnJwt::ValidateAndDecode::ValidateAndDecodeToken.new( - fetch_signing_key: mocked_fetch_signing_key_always_succeed, verify_and_decode_token: mocked_verify_and_decode_token_succeed_on_1st_time, fetch_jwt_claims_to_validate: mocked_fetch_jwt_claims_to_validate_valid, get_verification_option_by_jwt_claim: mocked_get_verification_option_by_jwt_claim_valid, - create_signing_key_provider: mocked_create_signing_key_provider_valid + create_signing_key_provider: mocked_create_signing_key_provider_always_succeed ).call( authenticator_input: authenticator_input, jwt_token: jwt_token_valid @@ -448,10 +408,9 @@ def valid_decoded_token(claims) context "and failed to fetch enforced claims" do subject do ::Authentication::AuthnJwt::ValidateAndDecode::ValidateAndDecodeToken.new( - fetch_signing_key: mocked_fetch_signing_key_always_succeed, verify_and_decode_token: mocked_verify_and_decode_token_succeed_on_1st_time, fetch_jwt_claims_to_validate: mocked_fetch_jwt_claims_to_validate_invalid, - create_signing_key_provider: mocked_create_signing_key_provider_valid + create_signing_key_provider: mocked_create_signing_key_provider_always_succeed ).call( authenticator_input: authenticator_input, jwt_token: jwt_token_valid @@ -467,10 +426,9 @@ def valid_decoded_token(claims) context "with empty claims list to validate" do subject do ::Authentication::AuthnJwt::ValidateAndDecode::ValidateAndDecodeToken.new( - fetch_signing_key: mocked_fetch_signing_key_always_succeed, verify_and_decode_token: mocked_verify_and_decode_token_succeed_on_1st_time, fetch_jwt_claims_to_validate: mocked_fetch_jwt_claims_to_validate_with_empty_claims, - create_signing_key_provider: mocked_create_signing_key_provider_valid + create_signing_key_provider: mocked_create_signing_key_provider_always_succeed ).call( authenticator_input: authenticator_input, jwt_token: jwt_token_valid @@ -485,10 +443,9 @@ def valid_decoded_token(claims) context "with mandatory claims which do not exist in token" do subject do ::Authentication::AuthnJwt::ValidateAndDecode::ValidateAndDecodeToken.new( - fetch_signing_key: mocked_fetch_signing_key_always_succeed, verify_and_decode_token: mocked_verify_and_decode_token_succeed_on_1st_time, fetch_jwt_claims_to_validate: mocked_fetch_jwt_claims_to_validate_with_not_exist_claims_in_token, - create_signing_key_provider: mocked_create_signing_key_provider_valid + create_signing_key_provider: mocked_create_signing_key_provider_always_succeed ).call( authenticator_input: authenticator_input, jwt_token: jwt_token_valid @@ -503,11 +460,10 @@ def valid_decoded_token(claims) context "and failed to get verification options" do subject do ::Authentication::AuthnJwt::ValidateAndDecode::ValidateAndDecodeToken.new( - fetch_signing_key: mocked_fetch_signing_key_always_succeed, verify_and_decode_token: mocked_verify_and_decode_token_succeed_on_1st_time, fetch_jwt_claims_to_validate: mocked_fetch_jwt_claims_to_validate_valid, get_verification_option_by_jwt_claim: mocked_get_verification_option_by_jwt_claim_invalid, - create_signing_key_provider: mocked_create_signing_key_provider_valid + create_signing_key_provider: mocked_create_signing_key_provider_always_succeed ).call( authenticator_input: authenticator_input, jwt_token: jwt_token_valid @@ -529,11 +485,10 @@ def valid_decoded_token(claims) context "and failed to validate claims" do subject do ::Authentication::AuthnJwt::ValidateAndDecode::ValidateAndDecodeToken.new( - fetch_signing_key: mocked_fetch_signing_key_always_succeed, verify_and_decode_token: mocked_verify_and_decode_token_failed_to_validate_claims, fetch_jwt_claims_to_validate: mocked_fetch_jwt_claims_to_validate_valid, get_verification_option_by_jwt_claim: mocked_get_verification_option_by_jwt_claim_valid, - create_signing_key_provider: mocked_create_signing_key_provider_valid + create_signing_key_provider: mocked_create_signing_key_provider_always_succeed ).call( authenticator_input: authenticator_input, jwt_token: jwt_token_valid @@ -549,11 +504,10 @@ def valid_decoded_token(claims) context "and keys are not updated" do subject do ::Authentication::AuthnJwt::ValidateAndDecode::ValidateAndDecodeToken.new( - fetch_signing_key: mocked_fetch_signing_key_always_succeed, verify_and_decode_token: mocked_verify_and_decode_token_succeed_to_validate_claims_when_keys_not_updated, fetch_jwt_claims_to_validate: mocked_fetch_jwt_claims_to_validate_valid, get_verification_option_by_jwt_claim: mocked_get_verification_option_by_jwt_claim_valid, - create_signing_key_provider: mocked_create_signing_key_provider_valid + create_signing_key_provider: mocked_create_signing_key_provider_always_succeed ).call( authenticator_input: authenticator_input, jwt_token: jwt_token_valid @@ -568,11 +522,10 @@ def valid_decoded_token(claims) context "and keys are updated" do subject do ::Authentication::AuthnJwt::ValidateAndDecode::ValidateAndDecodeToken.new( - fetch_signing_key: mocked_fetch_signing_key_always_succeed, verify_and_decode_token: mocked_verify_and_decode_token_succeed_to_validate_claims_when_keys_updated, fetch_jwt_claims_to_validate: mocked_fetch_jwt_claims_to_validate_valid, get_verification_option_by_jwt_claim: mocked_get_verification_option_by_jwt_claim_valid, - create_signing_key_provider: mocked_create_signing_key_provider_valid + create_signing_key_provider: mocked_create_signing_key_provider_always_succeed ).call( authenticator_input: authenticator_input, jwt_token: jwt_token_valid diff --git a/spec/app/domain/authentication/authn-jwt/validate_status_spec.rb b/spec/app/domain/authentication/authn-jwt/validate_status_spec.rb index e0e81c6247..ba916ecaa4 100644 --- a/spec/app/domain/authentication/authn-jwt/validate_status_spec.rb +++ b/spec/app/domain/authentication/authn-jwt/validate_status_spec.rb @@ -55,8 +55,6 @@ let(:account_does_not_exist_error) { "Account does not exist" } let(:identity_not_configured_properly) { "Identity not configured properly" } let(:mocked_valid_signing_key_provider) { double("Mocked valid signing key interface") } - let(:mocked_valid_fetch_signing_key) { double("Mocked valid fetch signing key interface") } - before(:each) do allow(mocked_valid_create_signing_key_provider).to( @@ -64,13 +62,9 @@ ) allow(mocked_valid_signing_key_provider).to( - receive(:signing_key_uri).and_return(valid_signing_key_uri) - ) - - allow(mocked_valid_fetch_signing_key).to( receive(:call).and_return(valid_signing_key) ) - + allow(mocked_invalid_create_signing_key_provider).to( receive(:call).and_raise(create_signing_key_configuration_is_invalid_error) ) @@ -93,7 +87,7 @@ allow(mocked_invalid_fetch_claim_aliases).to( receive(:call).and_raise(fetch_claim_aliases_configuration_is_invalid_error) ) - + allow(mocked_valid_identity_from_decoded_token_provider).to( receive(:new).and_return(mocked_valid_identity_configured_properly) ) @@ -162,7 +156,6 @@ subject do ::Authentication::AuthnJwt::ValidateStatus.new( - fetch_signing_key: mocked_valid_fetch_signing_key, create_signing_key_provider: mocked_valid_create_signing_key_provider, fetch_issuer_value: mocked_valid_fetch_issuer_value, identity_from_decoded_token_provider_class: mocked_valid_identity_from_decoded_token_provider, @@ -187,7 +180,6 @@ subject do ::Authentication::AuthnJwt::ValidateStatus.new( - fetch_signing_key: mocked_valid_fetch_signing_key, create_signing_key_provider: mocked_valid_create_signing_key_provider, fetch_issuer_value: mocked_valid_fetch_issuer_value, identity_from_decoded_token_provider_class: mocked_valid_identity_from_decoded_token_provider, @@ -211,7 +203,6 @@ subject do ::Authentication::AuthnJwt::ValidateStatus.new( - fetch_signing_key: mocked_valid_fetch_signing_key, create_signing_key_provider: mocked_valid_create_signing_key_provider, fetch_issuer_value: mocked_valid_fetch_issuer_value, identity_from_decoded_token_provider_class: mocked_valid_identity_from_decoded_token_provider, @@ -235,7 +226,6 @@ subject do ::Authentication::AuthnJwt::ValidateStatus.new( - fetch_signing_key: mocked_valid_fetch_signing_key, create_signing_key_provider: mocked_valid_create_signing_key_provider, fetch_issuer_value: mocked_valid_fetch_issuer_value, identity_from_decoded_token_provider_class: mocked_valid_identity_from_decoded_token_provider, @@ -259,7 +249,6 @@ subject do ::Authentication::AuthnJwt::ValidateStatus.new( - fetch_signing_key: mocked_valid_fetch_signing_key, create_signing_key_provider: mocked_valid_create_signing_key_provider, fetch_issuer_value: mocked_valid_fetch_issuer_value, identity_from_decoded_token_provider_class: mocked_valid_identity_from_decoded_token_provider, @@ -295,7 +284,6 @@ subject do ::Authentication::AuthnJwt::ValidateStatus.new( - fetch_signing_key: mocked_valid_fetch_signing_key, create_signing_key_provider: mocked_valid_create_signing_key_provider, fetch_issuer_value: mocked_valid_fetch_issuer_value, identity_from_decoded_token_provider_class: mocked_valid_identity_from_decoded_token_provider, @@ -320,7 +308,6 @@ context "signing key secrets are not configured properly" do subject do ::Authentication::AuthnJwt::ValidateStatus.new( - fetch_signing_key: mocked_valid_fetch_signing_key, create_signing_key_provider: mocked_invalid_create_signing_key_provider, fetch_issuer_value: mocked_valid_fetch_issuer_value, identity_from_decoded_token_provider_class: mocked_valid_identity_from_decoded_token_provider, @@ -343,7 +330,6 @@ context "issuer secrets are not configured properly" do subject do ::Authentication::AuthnJwt::ValidateStatus.new( - fetch_signing_key: mocked_valid_fetch_signing_key, create_signing_key_provider: mocked_valid_create_signing_key_provider, fetch_issuer_value: mocked_invalid_fetch_issuer_value, identity_from_decoded_token_provider_class: mocked_valid_identity_from_decoded_token_provider, @@ -366,7 +352,6 @@ context "audience secret is not configured properly" do subject do ::Authentication::AuthnJwt::ValidateStatus.new( - fetch_signing_key: mocked_valid_fetch_signing_key, create_signing_key_provider: mocked_valid_create_signing_key_provider, fetch_issuer_value: mocked_valid_fetch_issuer_value, fetch_audience_value: mocked_invalid_fetch_audience_value, @@ -390,7 +375,6 @@ context "enforced claims is not configured properly" do subject do ::Authentication::AuthnJwt::ValidateStatus.new( - fetch_signing_key: mocked_valid_fetch_signing_key, create_signing_key_provider: mocked_valid_create_signing_key_provider, fetch_issuer_value: mocked_valid_fetch_issuer_value, fetch_enforced_claims: mocked_invalid_fetch_enforced_claims, @@ -414,7 +398,6 @@ context "claim aliases is not configured properly" do subject do ::Authentication::AuthnJwt::ValidateStatus.new( - fetch_signing_key: mocked_valid_fetch_signing_key, create_signing_key_provider: mocked_valid_create_signing_key_provider, fetch_issuer_value: mocked_valid_fetch_issuer_value, fetch_claim_aliases: mocked_invalid_fetch_claim_aliases, @@ -438,7 +421,6 @@ context "identity secrets are not configured properly" do subject do ::Authentication::AuthnJwt::ValidateStatus.new( - fetch_signing_key: mocked_valid_fetch_signing_key, create_signing_key_provider: mocked_valid_create_signing_key_provider, fetch_issuer_value: mocked_valid_fetch_issuer_value, validate_identity_configured_properly: mocked_validate_identity_not_configured_properly, From 73672e223b8f65a4f525cf7d6875ad6053c14186 Mon Sep 17 00:00:00 2001 From: Sasha Chernomordik Date: Mon, 27 Dec 2021 19:52:20 +0200 Subject: [PATCH 2/2] Rename force_read parameter to force_fetch --- .../signing_key/fetch_jwks_uri_signing_key.rb | 4 ++-- .../signing_key/fetch_provider_uri_signing_key.rb | 4 ++-- .../validate_and_decode/validate_and_decode_token.rb | 6 +++--- app/domain/authentication/authn_jwt/validate_status.rb | 2 +- .../signing_key/fetch_jwks_signing_key_spec.rb | 10 +++++----- .../signing_key/fetch_provider_uri_signing_key_spec.rb | 8 ++++---- .../validate_and_decode_token_spec.rb | 10 +++++----- 7 files changed, 22 insertions(+), 22 deletions(-) diff --git a/app/domain/authentication/authn_jwt/signing_key/fetch_jwks_uri_signing_key.rb b/app/domain/authentication/authn_jwt/signing_key/fetch_jwks_uri_signing_key.rb index 4d7772b2b8..4ab9b4e93d 100644 --- a/app/domain/authentication/authn_jwt/signing_key/fetch_jwks_uri_signing_key.rb +++ b/app/domain/authentication/authn_jwt/signing_key/fetch_jwks_uri_signing_key.rb @@ -25,9 +25,9 @@ def initialize( @fetch_signing_key = fetch_signing_key end - def call(force_read:) + def call(force_fetch:) @fetch_signing_key.call( - refresh: force_read, + refresh: force_fetch, cache_key: jwks_uri, signing_key_provider: self ) diff --git a/app/domain/authentication/authn_jwt/signing_key/fetch_provider_uri_signing_key.rb b/app/domain/authentication/authn_jwt/signing_key/fetch_provider_uri_signing_key.rb index 795a6ea5a8..18cca4ba40 100644 --- a/app/domain/authentication/authn_jwt/signing_key/fetch_provider_uri_signing_key.rb +++ b/app/domain/authentication/authn_jwt/signing_key/fetch_provider_uri_signing_key.rb @@ -19,9 +19,9 @@ def initialize( @fetch_signing_key = fetch_signing_key end - def call(force_read:) + def call(force_fetch:) @fetch_signing_key.call( - refresh: force_read, + refresh: force_fetch, cache_key: provider_uri, signing_key_provider: self ) diff --git a/app/domain/authentication/authn_jwt/validate_and_decode/validate_and_decode_token.rb b/app/domain/authentication/authn_jwt/validate_and_decode/validate_and_decode_token.rb index fba7169ad4..9196f44124 100644 --- a/app/domain/authentication/authn_jwt/validate_and_decode/validate_and_decode_token.rb +++ b/app/domain/authentication/authn_jwt/validate_and_decode/validate_and_decode_token.rb @@ -41,9 +41,9 @@ def validate_token_exists raise Errors::Authentication::AuthnJwt::MissingToken if @jwt_token.blank? end - def fetch_signing_key(force_read: false) + def fetch_signing_key(force_fetch: false) @jwks = signing_key_provider.call( - force_read: force_read + force_fetch: force_fetch ) @logger.debug(LogMessages::Authentication::AuthnJwt::SigningKeysFetchedFromCache.new) end @@ -62,7 +62,7 @@ def ensure_keys_are_fresh LogMessages::Authentication::AuthnJwt::ValidateSigningKeysAreUpdated.new ) # maybe failed due to keys rotation. Force cache to read it again - fetch_signing_key(force_read: true) + fetch_signing_key(force_fetch: true) end def fetch_decoded_token_for_signature_only diff --git a/app/domain/authentication/authn_jwt/validate_status.rb b/app/domain/authentication/authn_jwt/validate_status.rb index 097dbb5b63..1a8b59bb51 100644 --- a/app/domain/authentication/authn_jwt/validate_status.rb +++ b/app/domain/authentication/authn_jwt/validate_status.rb @@ -141,7 +141,7 @@ def webservice def validate_signing_key signing_key_provider.call( - force_read: false + force_fetch: false ) @logger.debug(LogMessages::Authentication::AuthnJwt::ValidatedSigningKeyConfiguration.new) end diff --git a/spec/app/domain/authentication/authn-jwt/signing_key/fetch_jwks_signing_key_spec.rb b/spec/app/domain/authentication/authn-jwt/signing_key/fetch_jwks_signing_key_spec.rb index 0f7116638b..130971b8d6 100644 --- a/spec/app/domain/authentication/authn-jwt/signing_key/fetch_jwks_signing_key_spec.rb +++ b/spec/app/domain/authentication/authn-jwt/signing_key/fetch_jwks_signing_key_spec.rb @@ -93,7 +93,7 @@ fetch_authenticator_secrets: mocked_fetch_authenticator_secrets_exist_values, http_lib: mocked_bad_http_response, create_jwks_from_http_response: mocked_create_jwks_from_http_response - ).call(force_read: false) + ).call(force_fetch: false) end it "returns false" do @@ -109,7 +109,7 @@ fetch_authenticator_secrets: mocked_fetch_authenticator_secrets_exist_values, http_lib: mocked_bad_http_response, create_jwks_from_http_response: mocked_create_jwks_from_http_response - ).call(force_read: true) + ).call(force_fetch: true) end it "returns true" do @@ -125,7 +125,7 @@ fetch_authenticator_secrets: mocked_fetch_authenticator_secrets_exist_values, http_lib: mocked_bad_http_response, create_jwks_from_http_response: mocked_create_jwks_from_http_response - ).call(force_read: false) + ).call(force_fetch: false) end it "raises an error" do @@ -142,7 +142,7 @@ fetch_authenticator_secrets: mocked_fetch_authenticator_secrets_exist_values, http_lib: mocked_good_http_response, create_jwks_from_http_response: mocked_create_jwks_from_http_response - ).call(force_read: false) + ).call(force_fetch: false) end it "returns jwks value" do @@ -158,7 +158,7 @@ fetch_authenticator_secrets: mocked_fetch_authenticator_secrets_exist_values, http_lib: mocked_bad_http_response, create_jwks_from_http_response: mocked_create_jwks_from_http_response - ).call(force_read: false) + ).call(force_fetch: false) end it "raises an error" do diff --git a/spec/app/domain/authentication/authn-jwt/signing_key/fetch_provider_uri_signing_key_spec.rb b/spec/app/domain/authentication/authn-jwt/signing_key/fetch_provider_uri_signing_key_spec.rb index 37332814d2..1fb4411c41 100644 --- a/spec/app/domain/authentication/authn-jwt/signing_key/fetch_provider_uri_signing_key_spec.rb +++ b/spec/app/domain/authentication/authn-jwt/signing_key/fetch_provider_uri_signing_key_spec.rb @@ -86,7 +86,7 @@ logger: mocked_logger, fetch_authenticator_secrets: mocked_fetch_authenticator_secrets_exist_values, discover_identity_provider: mocked_discover_identity_provider - ).call(force_read: false) + ).call(force_fetch: false) end it "returns false" do @@ -101,7 +101,7 @@ logger: mocked_logger, fetch_authenticator_secrets: mocked_fetch_authenticator_secrets_exist_values, discover_identity_provider: mocked_discover_identity_provider - ).call(force_read: true) + ).call(force_fetch: true) end it "returns true" do @@ -118,7 +118,7 @@ logger: mocked_logger, fetch_authenticator_secrets: mocked_fetch_authenticator_secrets_exist_values, discover_identity_provider: mocked_invalid_uri_discover_identity_provider - ).call(force_read: false) + ).call(force_fetch: false) end it "raises an error" do @@ -133,7 +133,7 @@ logger: mocked_logger, fetch_authenticator_secrets: mocked_fetch_authenticator_secrets_exist_values, discover_identity_provider: mocked_discover_identity_provider - ).call(force_read: false) + ).call(force_fetch: false) end it "does not raise error" do diff --git a/spec/app/domain/authentication/authn-jwt/validate_and_decode/validate_and_decode_token_spec.rb b/spec/app/domain/authentication/authn-jwt/validate_and_decode/validate_and_decode_token_spec.rb index 4acc535f5b..813e011fd3 100644 --- a/spec/app/domain/authentication/authn-jwt/validate_and_decode/validate_and_decode_token_spec.rb +++ b/spec/app/domain/authentication/authn-jwt/validate_and_decode/validate_and_decode_token_spec.rb @@ -122,31 +122,31 @@ def valid_decoded_token(claims) allow(mocked_fetch_signing_key_provider_always_succeed).to( receive(:call).with( - force_read: false + force_fetch: false ).and_return(jwks_from_1st_call) ) allow(mocked_fetch_signing_key_provider_always_succeed).to( receive(:call).with( - force_read: true + force_fetch: true ).and_return(jwks_from_2nd_call) ) allow(mocked_fetch_signing_key_provider_failed_on_1st_time).to( receive(:call).with( - force_read: false + force_fetch: false ).and_raise(fetch_signing_key_1st_time_error) ) allow(mocked_fetch_signing_key_provider_failed_on_2nd_time).to( receive(:call).with( - force_read: false + force_fetch: false ).and_return(jwks_from_2nd_call) ) allow(mocked_fetch_signing_key_provider_failed_on_2nd_time).to( receive(:call).with( - force_read: true + force_fetch: true ).and_raise(fetch_signing_key_2nd_time_error) )