Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

authn-jwt - move cache binding point #2447

Merged
merged 2 commits into from
Dec 28, 2021
Merged

authn-jwt - move cache binding point #2447

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
20dc5c9
Signing key fetchers are using cache instead of upper level classes
sashaCher Dec 19, 2021
73672e2
Rename force_read parameter to force_fetch
sashaCher Dec 27, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions app/domain/authentication/authn_jwt/signing_key/create_signing_key_provider.rb
View file
Open in desktop
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
14 changes: 10 additions & 4 deletions app/domain/authentication/authn_jwt/signing_key/fetch_jwks_uri_signing_key.rb
View file
Open in desktop
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
14 changes: 10 additions & 4 deletions app/domain/authentication/authn_jwt/signing_key/fetch_provider_uri_signing_key.rb
View file
Open in desktop
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

sashaCher marked this conversation as resolved.
Show resolved Hide resolved
def discover_provider
Expand Down
20 changes: 4 additions & 16 deletions app/domain/authentication/authn_jwt/validate_and_decode/validate_and_decode_token.rb
View file
Open in desktop
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)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Authentication::AuthnJwt::ValidateAndDecode#fetch_signing_key has boolean parameter 'force_fetch'

@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
View file
Open in desktop
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
46 changes: 42 additions & 4 deletions spec/app/domain/authentication/authn-jwt/signing_key/fetch_jwks_signing_key_spec.rb
View file
Open in desktop
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
47 changes: 44 additions & 3 deletions spec/app/domain/authentication/authn-jwt/signing_key/fetch_provider_uri_signing_key_spec.rb
View file
Open in desktop
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