ONYX-11330: Fix cache of signing key

app/domain/authentication/authn_jwt/signing_key/fetch_cached_signing_key.rb

@@ -0,0 +1,27 @@
+module Authentication
+  module AuthnJwt
+    module SigningKey
+      # FetchCachedSigningKey is a wrapper of FetchSigningKeyInterface interface,
+      # in order to be able to store the signing key in our cache mechanism. If signing_key_interface don't have
+      # fetch_signing_key it is extreme case that error need to be raised so it can be investigated so reek will ignore
+      # this.
+      # :reek:InstanceVariableAssumption
+      class FetchCachedSigningKey
+        def initialize(logger: Rails.logger)
+          @logger = logger
+        end
+
+        def call(signing_key_interface:)
+          @signing_key_interface = signing_key_interface
+          fetch_signing_key
+        end
+
+        private
+
+        def fetch_signing_key
+          @signing_key_interface.fetch_signing_key
+        end
+      end
+    end
+  end
+end

app/domain/authentication/authn_jwt/signing_key/fetch_jwks_uri_signing_key.rb

@@ -31,11 +31,15 @@ def valid_configuration?
           @valid_configuration = jwks_uri_resource_exists?
         end
 
-        def call
+        def fetch_signing_key
           fetch_jwks_uri
           fetch_jwks_keys
         end
 
+        def signing_key_uri
+          jwks_uri
+        end
+
         private
 
         def jwks_uri_resource_exists?

app/domain/authentication/authn_jwt/signing_key/fetch_provider_uri_signing_key.rb

@@ -25,11 +25,15 @@ def valid_configuration?
           @valid_configuration = provider_uri_resource_exists?
         end
 
-        def call
+        def fetch_signing_key
           discover_provider
           fetch_provider_keys
         end
 
+        def signing_key_uri
+          provider_uri
+        end
+
         private
 
         def provider_uri_resource_exists?

app/domain/authentication/authn_jwt/signing_key/fetch_signing_key_interface.rb

@@ -2,7 +2,7 @@ module Authentication
   module AuthnJwt
     module SigningKey
       class FetchSigningKeyInterface
-        def call; end
+        def fetch_signing_key; end
 
         def valid_configuration?; end
       end

app/domain/authentication/authn_jwt/validate_and_decode/validate_and_decode_token.rb

@@ -7,19 +7,31 @@ module ValidateAndDecode
       # for the 2nd validation
       ValidateAndDecodeToken ||= CommandClass.new(
         dependencies: {
+          fetch_signing_key_from_cache: ::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,
+          signing_key_interface_factory: ::Authentication::AuthnJwt::SigningKey::CreateSigningKeyFactory.new,
           logger: Rails.logger
         },
-        inputs: %i[authentication_parameters fetch_signing_key]
+        inputs: %i[authentication_parameters]
       ) do
         extend(Forwardable)
         def_delegators(:@authentication_parameters, :jwt_token)
 
         def call
           @logger.debug(LogMessages::Authentication::AuthnJwt::ValidatingToken.new)
           validate_token_exists
+          fetch_signing_key_interface
           fetch_signing_key
           validate_signature
           fetch_jwt_claims_to_validate
@@ -31,12 +43,23 @@ def call
 
         private
 
+        # Don't do memoization here because otherwise interface won't change between different requests
+        def fetch_signing_key_interface
+          @signing_key_interface = @signing_key_interface_factory.call(
+            authentication_parameters: @authentication_parameters
+          )
+        end
+
         def validate_token_exists
           raise Errors::Authentication::AuthnJwt::MissingToken if jwt_token.blank?
         end
 
         def fetch_signing_key(force_read: false)
-          @jwks = @fetch_signing_key.call(refresh: force_read)
+          @jwks = @fetch_signing_key_from_cache.call(
+            refresh: force_read,
+            cache_key: @signing_key_interface.signing_key_uri,
+            signing_key_interface: @signing_key_interface
+          )
           @logger.debug(LogMessages::Authentication::AuthnJwt::SigningKeysFetchedFromCache.new)
         end
 

app/domain/authentication/authn_jwt/vendor_configurations/configuration_jwt_generic_vendor.rb

@@ -10,22 +10,22 @@ def initialize(
           logger: Rails.logger,
           authentication_parameters_class: Authentication::AuthnJwt::AuthenticationParameters,
           restriction_validator_class: Authentication::AuthnJwt::RestrictionValidation::ValidateRestrictionsOneToOne,
-          validate_and_decode_token_class: Authentication::AuthnJwt::ValidateAndDecode::ValidateAndDecodeToken,
           validate_resource_restrictions_class: Authentication::ResourceRestrictions::ValidateResourceRestrictions,
           extract_token_from_credentials: Authentication::AuthnJwt::InputValidation::ExtractTokenFromCredentials.new,
           create_identity_provider: Authentication::AuthnJwt::IdentityProviders::CreateIdentityProvider.new,
           create_constraints: Authentication::AuthnJwt::RestrictionValidation::CreateConstrains.new,
-          fetch_mapping_claims: Authentication::AuthnJwt::RestrictionValidation::FetchMappingClaims.new
+          fetch_mapping_claims: Authentication::AuthnJwt::RestrictionValidation::FetchMappingClaims.new,
+          validate_and_decode_token: Authentication::AuthnJwt::ValidateAndDecode::ValidateAndDecodeToken.new
         )
           @logger = logger
           @authentication_parameters_class = authentication_parameters_class
           @restriction_validator_class = restriction_validator_class
-          @validate_and_decode_token_class = validate_and_decode_token_class
           @validate_resource_restrictions_class = validate_resource_restrictions_class
           @extract_token_from_credentials = extract_token_from_credentials
           @create_identity_provider = create_identity_provider
           @create_constraints = create_constraints
           @fetch_mapping_claims = fetch_mapping_claims
+          @validate_and_decode_token = validate_and_decode_token
 
           @logger.debug(LogMessages::Authentication::AuthnJwt::CreatingAuthenticationParametersObject.new)
           @authentication_parameters = @authentication_parameters_class.new(
@@ -55,9 +55,8 @@ def validate_restrictions
         end
 
         def validate_and_decode_token
-          @authentication_parameters.decoded_token = validate_and_decode_token_instance.call(
-            authentication_parameters: @authentication_parameters,
-            fetch_signing_key: fetch_signing_key
+          @authentication_parameters.decoded_token = @validate_and_decode_token.call(
+            authentication_parameters: @authentication_parameters
           )
         end
 
@@ -83,27 +82,9 @@ def identity_provider
           )
         end
 
-        def validate_and_decode_token_instance
-          return @validate_and_decode_token_instance if @validate_and_decode_token_instance
-
-          @logger.debug(LogMessages::Authentication::AuthnJwt::CreateValidateAndDecodeTokenInstance.new)
-          @validate_and_decode_token_instance = @validate_and_decode_token_class.new(
-            fetch_jwt_claims_to_validate: fetch_jwt_claims_to_validate
-          )
-          @logger.debug(LogMessages::Authentication::AuthnJwt::CreatedValidateAndDecodeTokenInstance.new)
-          @validate_and_decode_token_instance
-        end
-
-        def fetch_signing_key
-          @fetch_signing_key ||= ::Util::ConcurrencyLimitedCache.new(
-            ::Util::RateLimitedCache.new(
-              fetch_signing_key_interface,
-              refreshes_per_interval: CACHE_REFRESHES_PER_INTERVAL,
-              rate_limit_interval: CACHE_RATE_LIMIT_INTERVAL,
-              logger: @logger
-            ),
-            max_concurrent_requests: CACHE_MAX_CONCURRENT_REQUESTS,
-            logger: @logger
+        def fetch_singing_key_interface
+          @fetch_singing_key_interface ||= create_signing_key_interface.call(
+            authentication_parameters: @authentication_parameters
           )
         end
 
@@ -124,10 +105,6 @@ def create_signing_key_interface
           @create_signing_key_interface ||= Authentication::AuthnJwt::SigningKey::CreateSigningKeyFactory.new
         end
 
-        def fetch_jwt_claims_to_validate
-          @fetch_jwt_claims_to_validate ||= ::Authentication::AuthnJwt::ValidateAndDecode::FetchJwtClaimsToValidate.new
-        end
-
         def restrictions_from_annotations
           @restrictions_from_annotations ||= Authentication::ResourceRestrictions::GetServiceSpecificRestrictionFromAnnotation.new
         end

app/domain/logs.rb

@@ -712,6 +712,8 @@ module AuthnJwt
     end
   end
 
+  # This are log messages so its okay there are many
+  # :reek:TooManyConstants
   module Util
 
     RateLimitedCacheUpdated = ::Util::TrackableLogMessageClass.new(
@@ -740,5 +742,15 @@ module Util
       code: "CONJ00023D"
     )
 
+    ConcurrencyLimitedCacheKeyRetrieved = ::Util::TrackableLogMessageClass.new(
+      msg: "ConcurrencyLimitedCache retrieved cache key : '{0-cache-key}'",
+      code: "CONJ00024D"
+    )
+
+    RateLimitedCacheKeyRetrieved = ::Util::TrackableLogMessageClass.new(
+      msg: "RateLimitedCache Retrieved cache key : '{0-cache-key}'",
+      code: "CONJ00025D"
+    )
+
   end
 end

app/domain/util/concurrency_limited_cache.rb

@@ -20,15 +20,20 @@ def initialize(
     # This  method is passed exactly the same named arguments you'd pass to the
     # callable object, but you can optionally include the `refresh: true` to
     # force recalculation.
+    #
+    # In this case we should call @cache[cache_key] because setting it to variable requires treatment of case cache_key
+    # not in case before the flow runs and it can cause unexpected behaviour so reek will ignore this.
+    # reek:DuplicateMethodCall
     def call(**args)
+      cache_key = cached_key(args)
       @concurrency_mutex.synchronize do
         if @concurrent_requests >= @max_concurrent_requests
           @logger.debug(
             LogMessages::Util::ConcurrencyLimitedCacheReached.new(@max_concurrent_requests)
           )
-          raise Errors::Util::ConcurrencyLimitReachedBeforeCacheInitialization unless @cache.key?(args)
+          raise Errors::Util::ConcurrencyLimitReachedBeforeCacheInitialization unless @cache.key?(cache_key)
 
-          return @cache[args]
+          return @cache[cache_key]
         end
 
         @concurrent_requests += 1
@@ -40,15 +45,15 @@ def call(**args)
       end
 
       @semaphore.synchronize do
-        recalculate(args)
-        @cache[args]
+        recalculate(args, cache_key)
+        @cache[cache_key]
       end
     end
 
     private
 
-    def recalculate(args)
-      @cache[args] = @target.call(**args)
+    def recalculate(args, cache_key)
+      @cache[cache_key] = @target.call(**args)
       @logger.debug(LogMessages::Util::ConcurrencyLimitedCacheUpdated.new)
       decrease_concurrent_requests
     rescue => e
@@ -66,5 +71,15 @@ def decrease_concurrent_requests
         )
       end
     end
+
+    def cached_key(args)
+      cache_key = args.key?(:cache_key) ? args.fetch(:cache_key) : args
+      @logger.debug(
+        LogMessages::Util::ConcurrencyLimitedCacheKeyRetrieved.new(
+          cache_key
+        )
+      )
+      cache_key
+    end
   end
 end

app/domain/util/rate_limited_cache.rb

@@ -43,17 +43,18 @@ def call(**args)
       args.delete(:refresh)
 
       @semaphore.synchronize do
-        first_calculation = [email protected]?(args)
-        recalculate(args) if refresh_requested || first_calculation
+        cache_key = cached_key(args)
+        first_calculation = [email protected]?(cache_key)
+        recalculate(args, cache_key) if refresh_requested || first_calculation
 
-        @cache[args]
+        @cache[cache_key]
       end
     end
 
     private
 
-    def recalculate(args)
-      if too_many_requests?(args)
+    def recalculate(args, cache_key)
+      if too_many_requests?(cache_key)
         @logger.debug(
           LogMessages::Util::RateLimitedCacheLimitReached.new(
             @refreshes_per_interval,
@@ -62,9 +63,24 @@ def recalculate(args)
         )
         return
       end
-      @cache[args] = @target.call(**args)
+      @cache[cache_key] = @target.call(**args)
       @logger.debug(LogMessages::Util::RateLimitedCacheUpdated.new)
-      @refresh_history[args].push(@time.now)
+      @refresh_history[cache_key].push(@time.now)
+    end
+
+    def cached_key(args)
+      if args.key?(:cache_key)
+        cache_key = args.fetch(:cache_key)
+        args.delete(:cache_key)
+        cache_key
+      end
+      cache_key = args
+      @logger.debug(
+        LogMessages::Util::RateLimitedCacheKeyRetrieved.new(
+          cache_key
+        )
+      )
+      cache_key
     end
 
     def too_many_requests?(args)

cucumber/authenticators_jwt/features/authn_jwt.feature

@@ -12,8 +12,6 @@ Feature: JWT Authenticator - JWKs Basic sanity
       id: conjur/authn-jwt/raw
       body:
       - !webservice
-        annotations:
-          description: Authentication service for JWT tokens, based on raw JWKs.
 
       - !variable
         id: jwks-uri
@@ -38,13 +36,14 @@ Feature: JWT Authenticator - JWKs Basic sanity
       member: !host myapp
     """
     And I am the super-user
-    And I successfully set authn-jwt jwks-uri variable with value of "myJWKs.json" endpoint
+    And I initialize remote JWKS endpoint with file "authn-jwt-general" and alg "RS256"
+    And I successfully set authn-jwt "jwks-uri" variable value to "http://jwks_py:8090/authn-jwt-general/RS256" in service "raw"
     And I successfully set authn-jwt "token-app-property" variable to value "host"
 
   @sanity
   Scenario: ONYX-8598: Authenticator is not enabled
     Given I have a "variable" resource called "test-variable"
-    And I issue a JWT token:
+    And I am using file "authn-jwt-general" and alg "RS256" for remotely issue token:
     """
     {
       "user":"myapp",
@@ -68,7 +67,7 @@ Feature: JWT Authenticator - JWKs Basic sanity
       annotations:
         authn-jwt/raw/custom-claim:
     """
-    And I issue a JWT token:
+    And I am using file "authn-jwt-general" and alg "RS256" for remotely issue token:
     """
     {
       "host":"not_premmited",