Skip to content

Commit

Permalink
ONYX-11330: Fix cache of signing key
Browse files Browse the repository at this point in the history
  • Loading branch information
tzheleznyak committed Aug 9, 2021
1 parent 5f35e14 commit 64ad246
Show file tree
Hide file tree
Showing 21 changed files with 328 additions and 359 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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

Expand All @@ -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

Expand All @@ -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
Expand Down
10 changes: 10 additions & 0 deletions app/domain/logs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -740,5 +740,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
23 changes: 17 additions & 6 deletions app/domain/util/concurrency_limited_cache.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,15 @@ def initialize(
# callable object, but you can optionally include the `refresh: true` to
# force recalculation.
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
Expand All @@ -40,15 +41,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
Expand All @@ -66,5 +67,15 @@ def decrease_concurrent_requests
)
end
end

def cached_key(args)
cache_key = args.has_key?(:cache_key) ? args.fetch(:cache_key) : args
@logger.debug(
LogMessages::Util::ConcurrencyLimitedCacheKeyRetrieved.new(
cache_key
)
)
cache_key
end
end
end
30 changes: 23 additions & 7 deletions app/domain/util/rate_limited_cache.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,17 +43,18 @@ def call(**args)
args.delete(:refresh)

@semaphore.synchronize do
first_calculation = !@cache.key?(args)
recalculate(args) if refresh_requested || first_calculation
cache_key = cached_key(args)
first_calculation = !@cache.key?(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,
Expand All @@ -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.has_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)
Expand Down
9 changes: 4 additions & 5 deletions cucumber/authenticators_jwt/features/authn_jwt.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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",
Expand All @@ -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",
Expand Down
Loading

0 comments on commit 64ad246

Please sign in to comment.