Skip to content

Commit

Permalink
Implement additional audit events
Browse files Browse the repository at this point in the history
Some API endpoints which return credentials do not write audit events.
This change adds the following audit events:
- `audit.service_instance.show` & `audit.user_provided_service_instance.show`
   Endpoints:
   - GET /v2/service_instances/:guid
   - GET /v3/service_instances/:guid/credentials

- `audit.service_binding.show`
  Endpoints:
  - GET /v3/service_credential_bindings/:guid/details
  - GET /v2/service_bindings/:guid

- `audit.service_key.show`
  Endpoints:
  - GET /v3/service_credential_bindings/:guid/details
  - GET /v2/service_keys/:guid

Co-authored-by: Wei Quan <[email protected]>
  • Loading branch information
johha and WeiQuan0605 committed Oct 24, 2023
1 parent e11485f commit 1e45a00
Show file tree
Hide file tree
Showing 11 changed files with 173 additions and 10 deletions.
13 changes: 9 additions & 4 deletions app/controllers/services/service_bindings_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,16 @@ class ServiceBindingsController < RestController::ModelController
get path_guid, :read

def read(guid)
obj = find_guid(guid)
raise CloudController::Errors::ApiError.new_from_details('ServiceBindingNotFound', guid) if obj.v2_app.blank?
service_binding = find_guid(guid)
raise CloudController::Errors::ApiError.new_from_details('ServiceBindingNotFound', guid) if service_binding.v2_app.blank?

validate_access(:read, obj)
object_renderer.render_json(self.class, obj, @opts)
validate_access(:read, service_binding)

Repositories::ServiceGenericBindingEventRepository.new(
Repositories::ServiceGenericBindingEventRepository::SERVICE_APP_CREDENTIAL_BINDING
).record_show(service_binding, UserAuditInfo.from_context(SecurityContext))

object_renderer.render_json(self.class, service_binding, @opts)
end

post path, :create
Expand Down
10 changes: 5 additions & 5 deletions app/controllers/services/service_instances_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -138,17 +138,17 @@ def update(guid)
status_code = status_from_operation_state(service_instance)
headers = { 'Location' => "#{self.class.path}/#{service_instance.guid}" } if status_code == HTTP::ACCEPTED

[
status_code,
headers,
object_renderer.render_json(self.class, service_instance, @opts)
]
[status_code, headers, object_renderer.render_json(self.class, service_instance, @opts)]
rescue LockCheck::ServiceBindingLockedError => e
raise CloudController::Errors::ApiError.new_from_details('AsyncServiceBindingOperationInProgress', e.service_binding.app.name, e.service_binding.service_instance.name)
end

def read(guid)
service_instance = find_guid_and_validate_access(:read, guid, ServiceInstance)

method = service_instance.instance_of?(UserProvidedServiceInstance) ? :record_user_provided_service_instance_event : :record_service_instance_event
@services_event_repository.send(method, :show, service_instance)

object_renderer.render_json(self.class, service_instance, @opts)
end

Expand Down
2 changes: 2 additions & 0 deletions app/controllers/services/service_keys_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ def read(guid)
e.name == 'NotAuthorized' ? raise(CloudController::Errors::ApiError.new_from_details('ServiceKeyNotFound', guid)) : raise(e)
end

@services_event_repository.record_service_key_event(:show, service_key)

[HTTP::OK,
{ 'Location' => "#{self.class.path}/#{service_key.guid}" },
object_renderer.render_json(self.class, service_key, @opts)]
Expand Down
12 changes: 11 additions & 1 deletion app/controllers/v3/service_credential_bindings_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ def details
not_found! unless can_read_secrets_in_the_binding_space?
not_found_with_message!(service_credential_binding) unless service_credential_binding.create_succeeded?

credentials = if service_credential_binding[:type] == 'key' && service_credential_binding.credhub_reference?
credentials = if service_credential_binding.is_a?(ServiceKey) && service_credential_binding.credhub_reference?
fetch_credentials_value(service_credential_binding.credhub_reference)
else
service_credential_binding.credentials
Expand All @@ -142,6 +142,16 @@ def details
credentials: credentials
).to_hash

type = if service_credential_binding.is_a?(ServiceKey)
Repositories::ServiceGenericBindingEventRepository::SERVICE_KEY_CREDENTIAL_BINDING
else
Repositories::ServiceGenericBindingEventRepository::SERVICE_APP_CREDENTIAL_BINDING
end
Repositories::ServiceGenericBindingEventRepository.new(type).record_show(
service_credential_binding,
user_audit_info
)

render status: :ok, json: details
end

Expand Down
2 changes: 2 additions & 0 deletions app/controllers/v3/service_instances_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,8 @@ def credentials
service_instance_not_found! unless service_instance && can_read_service_instance?(service_instance)
unauthorized! unless permission_queryer.can_read_secrets_in_space?(service_instance.space_id, service_instance.space.organization_id)

service_event_repository.record_user_provided_service_instance_event(:show, service_instance)

render status: :ok, json: (service_instance.credentials || {})
end

Expand Down
17 changes: 17 additions & 0 deletions app/repositories/service_generic_binding_event_repository.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,23 @@ def record_delete(service_binding, user_audit_info)
)
end

def record_show(service_binding, user_audit_info)
metadata = {
request: {
service_instance_guid: service_binding.service_instance_guid
}
}

metadata[:request].merge!({ app_guid: service_binding.app_guid }) if service_binding.try(:app_guid)

record_event(
type: "audit.#{@actee_name}.show",
service_binding: service_binding,
user_audit_info: user_audit_info,
metadata: metadata
)
end

private

def censor_request_attributes(request)
Expand Down
37 changes: 37 additions & 0 deletions spec/request/service_credential_bindings_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,25 @@ def check_filtered_bindings(*bindings)
api_call.call(admin_headers)
expect(last_response).to have_status_code(200)
end

it 'logs an audit event' do
api_call.call(admin_headers)
event = VCAP::CloudController::Event.find(type: 'audit.service_binding.show')

expect(event.type).to eq('audit.service_binding.show')
expect(event.actee_type).to eq('service_binding')

expect(event.metadata).to match(
{
'request' => {
'app_guid' => app_binding.app_guid,
'service_instance_guid' => app_binding.service_instance_guid
}
}
)

expect(event.actee).to eq(app_binding.guid)
end
end

context "last binding operation is in 'create in progress' state" do
Expand Down Expand Up @@ -792,6 +811,24 @@ def check_filtered_bindings(*bindings)
expect(credhub_server_stub).not_to have_been_requested
expect(parsed_response['credentials']).to eq(key_binding.credentials)
end

it 'logs an audit event' do
api_call.call(admin_headers)
event = VCAP::CloudController::Event.find(type: 'audit.service_key.show')
expect(event).to be

expect(event.actee).to eq(key_binding.guid)
expect(event.type).to eq('audit.service_key.show')
expect(event.actee_type).to eq('service_key')

expect(event.metadata).to match(
{
'request' => {
'service_instance_guid' => key_binding.service_instance_guid
}
}
)
end
end

context 'for app bindings with credhub references' do
Expand Down
23 changes: 23 additions & 0 deletions spec/request/service_instances_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,12 @@ def check_filtered_instances(*instances)
end

describe 'GET /v3/service_instances/:guid/credentials' do
let(:space_dev_headers) do
org.add_user(user)
space.add_developer(user)
headers_for(user)
end

it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS do
let(:api_call) { ->(user_headers) { get "/v3/service_instances/#{guid}/credentials", nil, user_headers } }
let(:credentials) { { 'fake-key' => 'fake-value' } }
Expand Down Expand Up @@ -526,6 +532,23 @@ def check_filtered_instances(*instances)
get "/v3/service_instances/#{msi.guid}/credentials", nil, admin_headers
expect(last_response).to have_status_code(404)
end

it 'records an audit event' do
upsi = VCAP::CloudController::UserProvidedServiceInstance.make(space: space, credentials: {})
get "/v3/service_instances/#{upsi.guid}/credentials", nil, space_dev_headers
expect(last_response).to have_status_code(200)

event = VCAP::CloudController::Event.last
expect(event.values).to include({
type: 'audit.user_provided_service_instance.show',
actor: user.guid,
actee: upsi.guid,
actee_type: 'user_provided_service_instance',
actee_name: upsi.name,
space_guid: space.guid,
organization_guid: space.organization.guid
})
end
end

describe 'GET /v3/service_instances/:guid/parameters' do
Expand Down
17 changes: 17 additions & 0 deletions spec/request/v2/service_bindings_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,23 @@
get "/v2/service_bindings/#{non_displayed_binding.guid}", nil, headers_for(user)
expect(last_response.status).to eq(404)
end

it 'creates an audit event' do
get "/v2/service_bindings/#{service_binding1.guid}", nil, headers_for(user)

event = VCAP::CloudController::Event.last
expect(event.type).to eq('audit.service_binding.show')
expect(event.actee).to eq(service_binding1.guid)
expect(event.actee_type).to eq('service_binding')
expect(event.metadata).to match(
{
'request' => {
'app_guid' => service_binding1.app_guid,
'service_instance_guid' => service_binding1.service_instance_guid
}
}
)
end
end

describe 'POST /v2/service_bindings' do
Expand Down
17 changes: 17 additions & 0 deletions spec/request/v2/service_keys_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,23 @@
}
)
end

it 'creates an audit event' do
get "/v2/service_keys/#{service_key1.guid}", nil, headers_for(user)

event = VCAP::CloudController::Event.last
expect(event.type).to eq('audit.service_key.show')
expect(event.actee).to eq(service_key1.guid)
expect(event.actee_type).to eq('service_key')
expect(event.metadata).to match(
{
'request' => {
'name' => service_key1.name,
'service_instance_guid' => service_key1.service_instance_guid
}
}
)
end
end

describe 'GET /v2/service_keys/:guid/parameters' do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## NOTICE: Prefer request specs over controller specs as per ADR #0003 ##

# rubocop:disable Metrics/ModuleLength
module VCAP::CloudController
RSpec.describe VCAP::CloudController::ServiceInstancesController, :services do
let(:service_broker_url_regex) { %r{http://example.com/v2/service_instances/(.*)} }
Expand Down Expand Up @@ -1563,6 +1564,22 @@ def stub_delete_and_return(status, body)
expect(last_response.status).to eq(200)
expect(decoded_response.fetch('metadata').fetch('guid')).to eq(service_instance.guid)
end

it 'creates a service audit event for showing the service instance' do
set_current_user(developer)
get "v2/service_instances/#{service_instance.guid}"

event = VCAP::CloudController::Event.first(type: 'audit.service_instance.show')
expect(event.type).to eq('audit.service_instance.show')
expect(event.actor_type).to eq('user')
expect(event.actor).to eq(developer.guid)
expect(event.timestamp).to be
expect(event.actee).to eq(service_instance.guid)
expect(event.actee_type).to eq('service_instance')
expect(event.actee_name).to eq(service_instance.name)
expect(event.space_guid).to eq(service_instance.space.guid)
expect(event.organization_guid).to eq(service_instance.space.organization.guid)
end
end

context 'with a user provided service instance' do
Expand All @@ -1581,6 +1598,21 @@ def stub_delete_and_return(status, body)
expect(last_response.status).to eq(200)
expect(decoded_response.fetch('entity').fetch('service_bindings_url')).to include('user_provided_service_instances')
end

it 'creates a service audit event for showing the service instance' do
get "v2/service_instances/#{service_instance.guid}"

event = VCAP::CloudController::Event.first(type: 'audit.user_provided_service_instance.show')
expect(event.type).to eq('audit.user_provided_service_instance.show')
expect(event.actor_type).to eq('user')
expect(event.actor).to eq(developer.guid)
expect(event.timestamp).to be
expect(event.actee).to eq(service_instance.guid)
expect(event.actee_type).to eq('user_provided_service_instance')
expect(event.actee_name).to eq(service_instance.name)
expect(event.space_guid).to eq(service_instance.space.guid)
expect(event.organization_guid).to eq(service_instance.space.organization.guid)
end
end
end

Expand Down Expand Up @@ -5482,3 +5514,4 @@ def max_tags
end
end
end
# rubocop:enable Metrics/ModuleLength

0 comments on commit 1e45a00

Please sign in to comment.