Skip to content

Commit

Permalink
Merge pull request #2447 from cyberark/authn-jwt-move-cache-layer
Browse files Browse the repository at this point in the history
ONYX-15138 - authn-jwt - move cache binding point
  • Loading branch information
sashaCher authored Dec 28, 2021
2 parents 29aacaa + 73672e2 commit 2c93793
Show file tree
Hide file tree
Showing 9 changed files with 171 additions and 156 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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

Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -21,17 +22,22 @@ def initialize(
@fetch_authenticator_secrets = fetch_authenticator_secrets

@authenticator_input = authenticator_input
@fetch_signing_key = fetch_signing_key
end

def call(force_fetch:)
@fetch_signing_key.call(
refresh: force_fetch,
cache_key: jwks_uri,
signing_key_provider: self
)
end

def fetch_signing_key
fetch_jwks_uri
fetch_jwks_keys
end

def signing_key_uri
jwks_uri
end

private

def fetch_jwks_uri
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -15,17 +16,22 @@ def initialize(
@discover_identity_provider = discover_identity_provider

@authenticator_input = authenticator_input
@fetch_signing_key = fetch_signing_key
end

def call(force_fetch:)
@fetch_signing_key.call(
refresh: force_fetch,
cache_key: provider_uri,
signing_key_provider: self
)
end

def fetch_signing_key
discover_provider
fetch_provider_keys
end

def signing_key_uri
provider_uri
end

private

def discover_provider
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -51,11 +41,9 @@ 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,
cache_key: signing_key_provider.signing_key_uri,
signing_key_provider: signing_key_provider
def fetch_signing_key(force_fetch: false)
@jwks = signing_key_provider.call(
force_fetch: force_fetch
)
@logger.debug(LogMessages::Authentication::AuthnJwt::SigningKeysFetchedFromCache.new)
end
Expand All @@ -74,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
Expand Down
15 changes: 2 additions & 13 deletions app/domain/authentication/authn_jwt/validate_status.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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_fetch: false
)
@logger.debug(LogMessages::Authentication::AuthnJwt::ValidatedSigningKeyConfiguration.new)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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") }
Expand All @@ -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')
)
Expand Down Expand Up @@ -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_fetch: 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_fetch: 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_fetch: false)
end

it "raises an error" do
Expand All @@ -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_fetch: false)
end

it "returns jwks value" do
Expand All @@ -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_fetch: false)
end

it "raises an error" do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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") }
Expand All @@ -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')
)
Expand All @@ -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_fetch: 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_fetch: 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_fetch: false)
end

it "raises an error" do
Expand All @@ -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_fetch: false)
end

it "does not raise error" do
Expand Down
Loading

0 comments on commit 2c93793

Please sign in to comment.