Skip to content

Commit

Permalink
Raise proper error in case k8s host not found
Browse files Browse the repository at this point in the history
- Added RoleNotFound error message in case host not exist for hostId
- Fixed bug when creating InjectClientCert inside log write command
- Added UT
  • Loading branch information
mbenita-Cyberark committed Feb 22, 2021
1 parent c7bb95a commit 4359d63
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 3 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

## [Unreleased]
### Changed
- Conjur now raises a RoleNotFound when trying to authenticate a non-existing host in authn-k8s
[conjurinc/appliance#1546]https://github.com/conjurinc/appliance/issues/1546

## [1.11.2] - 2021-02-02
### Added
Expand Down
20 changes: 17 additions & 3 deletions app/domain/authentication/authn_k8s/inject_client_cert.rb
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,13 @@ def spiffe_id
end

def host
@host ||= @resource_class[host_id]
return @host if @host

@host = @resource_class[host_id]

raise Errors::Authentication::Security::RoleNotFound, host_id unless @host

@host
end

def validate_spiffe_id_exists
Expand Down Expand Up @@ -161,7 +167,7 @@ def audit_success
Audit::Event::Authn::InjectClientCert.new(
authenticator_name: AUTHENTICATOR_NAME,
service: webservice,
role_id: host.id,
role_id: sanitized_role_id,
client_ip: @client_ip,
success: true,
error_message: nil
Expand All @@ -174,13 +180,21 @@ def audit_failure(err)
Audit::Event::Authn::InjectClientCert.new(
authenticator_name: AUTHENTICATOR_NAME,
service: webservice,
role_id: host.id,
role_id: sanitized_role_id,
client_ip: @client_ip,
success: false,
error_message: err.message
)
)
end
# TODO: this logic is taken from app/models/audit/event/authn.rb.
# We should use that logic here.
# Masking role if it doesn't exist to avoid audit pollution
def sanitized_role_id
return "not-found" unless @resource_class[host_id]

host.id
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
let(:csr) { "CSR" }

let(:host_id) { "HostId" }
let(:non_existing_host_id) { "non-exist" }
let(:host_role) { double("HostRole", id: host_id) }

let(:k8s_authn_container_name) { 'kubernetes/authentication-container-name' }
Expand Down Expand Up @@ -49,6 +50,14 @@
)
}

let(:non_existing_k8s_host) {
double(
"K8sHost",
account: account,
conjur_host_id: non_existing_host_id
)
}

let(:spiffe_altname) { "SpiffeAltname" }
let(:spiffe_id) {
double(
Expand Down Expand Up @@ -105,6 +114,10 @@
allow(resource_class).to receive(:[])
.with(host_id)
.and_return(host)

allow(resource_class).to receive(:[])
.with(non_existing_host_id)
.and_return(nil)
end
end

Expand Down Expand Up @@ -222,6 +235,37 @@
end
end

context "when host does not exist" do
it "throws RoleNotFound" do
error_type = Errors::Authentication::Security::RoleNotFound

allow(Authentication::AuthnK8s::K8sHost)
.to receive(:from_csr)
.with(hash_including(account: account,
service_name: service_id,
csr: anything))
.and_return(non_existing_k8s_host)

allow(Authentication::AuthnK8s::PodRequest)
.to receive(:new)
.with(hash_including(service_id: service_id,
k8s_host: non_existing_k8s_host,
spiffe_id: spiffe_id))
.and_return(pod_request)

allow(validate_pod_request)
.to receive(:call)
.with(hash_including(pod_request: anything))
.and_return(nil)

expect { injector.(conjur_account: account,
service_id: service_id,
csr: csr,
host_id_prefix: host_id_prefix,
client_ip: client_ip) }.to raise_error(error_type)
end
end

context "when validate_pod_request fails" do
let(:audit_success) { false }

Expand Down

0 comments on commit 4359d63

Please sign in to comment.