Skip to content

Commit 69a6f88

Browse files
committed
Final bits to get all cucumber tests passing
1 parent 39a86c0 commit 69a6f88

File tree

8 files changed

+375
-350
lines changed

8 files changed

+375
-350
lines changed

app/db/repository/authenticator_repository.rb

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,14 +63,16 @@ def load_authenticator(type:, account:, service_id:)
6363
"#{account}:variable:conjur/#{type}/#{service_id}/%"
6464
)
6565
).eager(:secrets).all
66-
6766
args_list = {}.tap do |args|
6867
args[:account] = account
6968
args[:service_id] = service_id
7069
variables.each do |variable|
71-
next unless variable.secret
7270

73-
args[variable.resource_id.split('/')[-1].underscore.to_sym] = variable.secret.value
71+
# If variable exists but does not have a secret, set the value to an empty string.
72+
# This is used downstream for validating if a variable has been set or not, and thus,
73+
# what error to raise.
74+
value = variable.secret ? variable.secret.value : ''
75+
args[variable.resource_id.split('/')[-1].underscore.to_sym] = value
7476
end
7577
end
7678

@@ -79,7 +81,8 @@ def load_authenticator(type:, account:, service_id:)
7981
allowed_args = %i[account service_id] +
8082
@data_object.const_get(:REQUIRED_VARIABLES) +
8183
@data_object.const_get(:OPTIONAL_VARIABLES)
82-
args_list = args_list.select{ |key, value| allowed_args.include?(key) && value.present? }
84+
# Ensure only defined parameters are added
85+
args_list = args_list.select{ |key, _| allowed_args.include?(key) }
8386
end
8487
@data_object.new(**args_list)
8588
rescue ArgumentError => e

app/domain/authentication/authn_jwt/v2/data_objects/authenticator.rb

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,12 @@ class Authenticator
2020
:ca_cert,
2121
:identity_path,
2222
:issuer,
23-
:claim_aliases,
2423
:audience
2524

2625
# moved to methods below to allow for "validation"
2726
# :enforced_claims,
2827
# :token_app_property,
28+
# :claim_aliases,
2929
)
3030

3131
def initialize(
@@ -54,7 +54,7 @@ def initialize(
5454
@issuer = issuer
5555
@enforced_claims = enforced_claims
5656
# ensure we have a string so we can split safely to generate the hash lookup
57-
@claim_aliases = claim_aliases.to_s
57+
@claim_aliases = claim_aliases
5858
@audience = audience
5959
@token_ttl = token_ttl
6060
end
@@ -69,6 +69,15 @@ def token_ttl
6969
raise Errors::Authentication::DataObjects::InvalidTokenTTL.new(resource_id, @token_ttl)
7070
end
7171

72+
def claim_aliases
73+
if @claim_aliases == ''
74+
raise Errors::Conjur::RequiredSecretMissing,
75+
"#{@account}:variable:conjur/authn-jwt/#{@service_id}/claim-aliases"
76+
end
77+
78+
@claim_aliases
79+
end
80+
7281
def token_app_property
7382
token_app_property = @token_app_property.to_s
7483
# Ensure claim contain only "allowed" characters (alpha-numeric, and: "-", "_", "/", ".")
@@ -81,6 +90,11 @@ def token_app_property
8190
end
8291

8392
def enforced_claims
93+
if @enforced_claims == ''
94+
raise Errors::Conjur::RequiredSecretMissing,
95+
"#{@account}:variable:conjur/authn-jwt/#{@service_id}/enforced-claims"
96+
end
97+
8498
@claims ||= begin
8599
claims = @enforced_claims.to_s.split(',').map(&:strip)
86100

@@ -105,8 +119,8 @@ def denylist
105119
def claim_aliases_lookup
106120
@claim_aliases_lookup ||= begin
107121
{}.tap do |rtn|
108-
@claim_aliases.split(',').each do |claim_alias|
109-
key, value = claim_alias.split(':').map(&:strip)
122+
@claim_aliases.to_s.split(',').each do |claim_alias|
123+
key, value = claim_alias.to_s.split(':').map(&:strip)
110124

111125
# If alias is defined multiple times
112126
if rtn.key?(key)

app/domain/authentication/authn_jwt/v2/strategy.rb

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,27 @@ def callback(parameters:, request_body:)
3737
"jwks-uri and provider-uri cannot be defined simultaneously"
3838
end
3939

40+
if @authenticator.issuer == ''
41+
raise Errors::Conjur::RequiredSecretMissing,
42+
"#{@authenticator.account}:variable:conjur/authn-jwt/#{@authenticator.service_id}/issuer"
43+
end
44+
45+
# Need to trigger an error if this variable is present, but has not been set...
46+
# TODO: fix with validation
47+
@authenticator.claim_aliases
48+
# binding.pry
49+
50+
if @authenticator.jwks_uri == '' && @authenticator.provider_uri.nil? && @authenticator.public_keys.nil?
51+
raise Errors::Conjur::RequiredSecretMissing,
52+
"#{@authenticator.account}:variable:conjur/authn-jwt/#{@authenticator.service_id}/jwks-uri"
53+
end
54+
4055
if @authenticator.jwks_uri.blank? && @authenticator.provider_uri.blank? && @authenticator.public_keys.blank?
56+
if @authenticator.jwks_uri.nil? && @authenticator.provider_uri.nil? && @authenticator.public_keys.nil?
57+
raise Errors::Authentication::AuthnJwt::InvalidSigningKeySettings,
58+
'One of the following must be defined: jwks-uri, public-keys, or provider-uri'
59+
end
60+
4161
raise Errors::Authentication::AuthnJwt::InvalidSigningKeySettings,
4262
'Failed to find a JWT decode option. Either `jwks-uri` or `public-keys` variable must be set'
4363
end
@@ -64,6 +84,8 @@ def callback(parameters:, request_body:)
6484
keys = @json.parse(@authenticator.public_keys)&.deep_symbolize_keys
6585
hash[:jwks] = keys[:value]
6686
elsif @authenticator.provider_uri.present?
87+
# If we're validating with Provider URI, it means we're operating
88+
# against an OIDC enpoint.
6789
begin
6890
if @authenticator.issuer.present?
6991
hash[:iss] = @authenticator.issuer

app/domain/authentication/handler/authentication_handler.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,10 @@ def call(request_ip:, parameters: nil, request_body: nil, action: nil)
123123
end
124124

125125
def handle_error(err)
126+
# Log authentication errors (but don't raise...)
127+
authentication_error = LogMessages::Authentication::AuthenticationError.new(err.inspect)
128+
@logger.info(authentication_error)
129+
126130
@logger.info("#{err.class.name}: #{err.message}")
127131
err.backtrace.each {|l| @logger.info(l) }
128132

0 commit comments

Comments
 (0)