Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Merge pull request #339 from xingzhou/service_key

Add "create-service-key" to cloud controller, based on story #87057732
  • Loading branch information...
commit 45765e029df787f0f6d0a53a74a9f1b411fd1afc 2 parents 48126c9 + bb4a58e
@dsabeti dsabeti authored
View
13 app/access/service_key_access.rb
@@ -0,0 +1,13 @@
+module VCAP::CloudController
+ class ServiceKeyAccess < BaseAccess
+ def create?(service_key, params=nil)
+ return true if admin_user?
+ return false if service_key.in_suspended_org?
+ service_key.service_instance.space.developers.include?(context.user)
+ end
+
+ def delete?(service_key)
+ create?(service_key)
+ end
+ end
+end
View
62 app/controllers/services/lifecycle/service_key_manager.rb
@@ -0,0 +1,62 @@
+module VCAP::CloudController
+ class ServiceKeyManager
+ class ServiceInstanceNotFound < StandardError; end
+ class ServiceInstanceNotBindable < StandardError; end
+
+ def initialize(services_event_repository, access_validator, logger)
+ @services_event_repository = services_event_repository
+ @access_validator = access_validator
+ @logger = logger
+ end
+
+ def create_service_key(request_attrs)
+ service_instance = ServiceInstance.first(guid: request_attrs['service_instance_guid'])
+ raise ServiceInstanceNotFound unless service_instance
+ raise ServiceInstanceNotBindable unless service_instance.bindable?
+
+ service_key = ServiceKey.new(request_attrs)
+ @access_validator.validate_access(:create, service_key)
+ raise Sequel::ValidationFailed.new(service_key) unless service_key.valid?
+
+ lock_service_instance_by_blocking(service_instance) do
+ attributes_to_update = service_key.client.bind(service_key)
+ begin
+ service_key.set_all(attributes_to_update)
+ service_key.save
+ rescue
+ safe_unbind_instance(service_key)
+ raise
+ end
+ end
+
+ service_key
+ end
+
+ private
+
+ def safe_unbind_instance(service_key)
+ service_key.client.unbind(service_key)
+ rescue => e
+ @logger.error "Unable to unbind #{service_key}: #{e}"
+ end
+
+ def lock_service_instance_by_blocking(service_instance, &block)
+ return block.call unless service_instance.managed_instance?
+
+ original_attributes = service_instance.last_operation.try(:to_hash)
+ begin
+ service_instance.lock_by_failing_other_operations('update') do
+ block.call
+ end
+ ensure
+ if original_attributes
+ service_instance.last_operation.set_all(original_attributes)
+ service_instance.last_operation.save
+ else
+ service_instance.service_instance_operation.destroy
+ service_instance.save
+ end
+ end
+ end
+ end
+end
View
42 app/controllers/services/service_keys_controller.rb
@@ -0,0 +1,42 @@
+require 'services/api'
+
+module VCAP::CloudController
+ class ServiceKeysController < RestController::ModelController
+ define_attributes do
+ to_one :service_instance
+ attribute :name, String
+ end
+
+ post path, :create
+ def create
+ @request_attrs = self.class::CreateMessage.decode(body).extract(stringify_keys: true)
+ logger.debug 'cc.create', model: self.class.model_class_name, attributes: request_attrs
+ raise InvalidRequest unless request_attrs
+ service_key_manager = ServiceKeyManager.new(@services_event_repository, self, logger)
+ service_key = service_key_manager.create_service_key(@request_attrs)
+ [HTTP::CREATED,
+ { 'Location' => "#{self.class.path}/#{service_key.guid}" },
+ object_renderer.render_json(self.class, service_key, @opts)
+ ]
+ rescue ServiceKeyManager::ServiceInstanceNotFound
+ raise VCAP::Errors::ApiError.new_from_details('ServiceInstanceNotFound', @request_attrs['service_instance_guid'])
+ rescue ServiceKeyManager::ServiceInstanceNotBindable
+ raise VCAP::Errors::ApiError.new_from_details('UnbindableService')
+ end
+
+ private
+
+ def self.translate_validation_exception(e, attributes)
+ unique_errors = e.errors.on([:name, :service_instance_id])
+ if unique_errors && unique_errors.include?(:unique)
+ Errors::ApiError.new_from_details('ServiceKeyTaken', "#{attributes['name']} #{attributes['service_instance_guid']}")
+ elsif e.errors.on(:service_instance) && e.errors.on(:service_instance).include?(:presence)
+ Errors::ApiError.new_from_details('ServiceInstanceNotFound', attributes['service_instance_guid'])
+ else
+ Errors::ApiError.new_from_details('ServiceKeyInvalid', e.errors.full_messages)
+ end
+ end
+
+ define_messages
+ end
+end
View
1  app/models.rb
@@ -58,6 +58,7 @@
require 'models/services/service_create_event'
require 'models/services/service_delete_event'
require 'models/services/service_usage_event'
+require 'models/services/service_key'
require 'models/job'
View
71 app/models/services/service_key.rb
@@ -0,0 +1,71 @@
+module VCAP::CloudController
+ class ServiceKey < Sequel::Model
+ class InvalidAppAndServiceRelation < StandardError; end
+
+ many_to_one :service_instance
+
+ export_attributes :name, :service_instance_guid, :credentials
+
+ import_attributes :name, :service_instance_guid, :credentials
+
+ delegate :client, :service, :service_plan, to: :service_instance
+
+ plugin :after_initialize
+
+ encrypt :credentials, salt: :salt
+
+ def to_hash(opts={})
+ if !VCAP::CloudController::SecurityContext.admin? && !service_instance.space.developers.include?(VCAP::CloudController::SecurityContext.current_user)
+ opts.merge!({ redact: ['credentials'] })
+ end
+ super(opts)
+ end
+
+ def in_suspended_org?
+ space.in_suspended_org?
+ end
+
+ def space
+ service_instance.space
+ end
+
+ def validate
+ validates_presence :name
+ validates_presence :service_instance
+ validates_unique [:name, :service_instance_id]
+ end
+
+ def credentials_with_serialization=(val)
+ self.credentials_without_serialization = MultiJson.dump(val)
+ end
+ alias_method_chain :credentials=, 'serialization'
+
+ def credentials_with_serialization
+ string = credentials_without_serialization
+ return if string.blank?
+ MultiJson.load string
+ end
+ alias_method_chain :credentials, 'serialization'
+
+ def self.user_visibility_filter(user)
+ { service_instance: ServiceInstance.user_visible(user) }
+ end
+
+ def after_initialize
+ super
+ self.guid ||= SecureRandom.uuid
+ end
+
+ def logger
+ @logger ||= Steno.logger('cc.models.service_key')
+ end
+
+ private
+
+ def safe_unbind
+ client.unbind(self)
+ rescue => unbind_e
+ logger.error "Unable to unbind #{self}: #{unbind_e}"
+ end
+ end
+end
View
13 db/migrations/20150316184259_create_service_key_table.rb
@@ -0,0 +1,13 @@
+Sequel.migration do
+ change do
+ create_table(:service_keys) do
+ VCAP::Migration.common(self, :sk)
+ String :name, null: false
+ String :salt
+ String :credentials, null: false, size: 2048
+ Integer :service_instance_id, null: false
+ foreign_key [:service_instance_id], :service_instances, name: :fk_svc_key_svc_instance_id
+ index [:name, :service_instance_id], unique: true, name: :svc_key_name_instance_id_index
+ end
+ end
+end
View
14 lib/services/service_brokers/v2/client.rb
@@ -92,11 +92,15 @@ def fetch_service_instance_state(instance)
def bind(binding)
path = service_binding_resource_path(binding)
- response = @http_client.put(path, {
- service_id: binding.service.broker_provided_id,
- plan_id: binding.service_plan.broker_provided_id,
- app_guid: binding.app_guid
- })
+ attr = {
+ service_id: binding.service.broker_provided_id,
+ plan_id: binding.service_plan.broker_provided_id
+ }
+ if binding.respond_to? 'app_guid'
+ attr[:app_guid] = binding.app_guid
+ end
+
+ response = @http_client.put(path, attr)
parsed_response = @response_parser.parse(:put, path, response)
attributes = {
View
6 spec/support/fakes/blueprints.rb
@@ -194,6 +194,12 @@ module VCAP::CloudController
syslog_drain_url { nil }
end
+ ServiceKey.blueprint do
+ credentials { Sham.service_credentials }
+ service_instance { ManagedServiceInstance.make }
+ name { Sham.name }
+ end
+
ServiceBroker.blueprint do
name { Sham.name }
broker_url { Sham.url }
View
120 spec/unit/access/service_key_access_spec.rb
@@ -0,0 +1,120 @@
+require 'spec_helper'
+
+module VCAP::CloudController
+ describe ServiceKeyAccess, type: :access do
+ subject(:access) { ServiceKeyAccess.new(Security::AccessContext.new) }
+ let(:token) { { 'scope' => ['cloud_controller.read', 'cloud_controller.write'] } }
+
+ let(:user) { VCAP::CloudController::User.make }
+ let(:service) { VCAP::CloudController::Service.make }
+ let(:org) { VCAP::CloudController::Organization.make }
+ let(:space) { VCAP::CloudController::Space.make(organization: org) }
+ let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space: space) }
+
+ let(:object) { VCAP::CloudController::ServiceKey.make(name: 'fake-key', service_instance: service_instance) }
+
+ before do
+ SecurityContext.set(user, token)
+ end
+
+ after do
+ SecurityContext.clear
+ end
+
+ it_should_behave_like :admin_full_access
+
+ context 'for a logged in user (defensive)' do
+ it_behaves_like :no_access
+ end
+
+ context 'a user that isnt logged in (defensive)' do
+ let(:user) { nil }
+ it_behaves_like :no_access
+ end
+
+ context 'organization manager (defensive)' do
+ before { org.add_manager(user) }
+ it_behaves_like :no_access
+ end
+
+ context 'organization billing manager (defensive)' do
+ before { org.add_billing_manager(user) }
+ it_behaves_like :no_access
+ end
+
+ context 'organization auditor (defensive)' do
+ before { org.add_auditor(user) }
+ it_behaves_like :no_access
+ end
+
+ context 'organization user (defensive)' do
+ before { org.add_user(user) }
+ it_behaves_like :no_access
+ end
+
+ context 'space auditor' do
+ before do
+ org.add_user(user)
+ space.add_auditor(user)
+ end
+
+ it_behaves_like :read_only
+ end
+
+ context 'space manager (defensive)' do
+ before do
+ org.add_user(user)
+ space.add_manager(user)
+ end
+
+ it_behaves_like :no_access
+ end
+
+ context 'space developer' do
+ before do
+ org.add_user(user)
+ space.add_developer(user)
+ end
+
+ it { is_expected.to allow_op_on_object :create, object }
+ it { is_expected.to allow_op_on_object :read, object }
+ it { is_expected.not_to allow_op_on_object :read_for_update, object }
+ it { is_expected.to allow_op_on_object :delete, object }
+
+ context 'when the organization is suspended' do
+ before { allow(object).to receive(:in_suspended_org?).and_return(true) }
+ it_behaves_like :read_only
+ end
+ end
+
+ context 'any user using client without cloud_controller.write' do
+ let(:token) { { 'scope' => ['cloud_controller.read'] } }
+ before do
+ org.add_user(user)
+ org.add_manager(user)
+ org.add_billing_manager(user)
+ org.add_auditor(user)
+ space.add_manager(user)
+ space.add_developer(user)
+ space.add_auditor(user)
+ end
+
+ it_behaves_like :read_only
+ end
+
+ context 'any user using client without cloud_controller.read' do
+ let(:token) { { 'scope' => [] } }
+ before do
+ org.add_user(user)
+ org.add_manager(user)
+ org.add_billing_manager(user)
+ org.add_auditor(user)
+ space.add_manager(user)
+ space.add_developer(user)
+ space.add_auditor(user)
+ end
+
+ it_behaves_like :no_access
+ end
+ end
+end
View
215 spec/unit/controllers/services/service_keys_controller_spec.rb
@@ -0,0 +1,215 @@
+require 'spec_helper'
+
+module VCAP::CloudController
+ describe ServiceKeysController do
+ describe 'Attributes' do
+ it do
+ expect(described_class).to have_creatable_attributes({
+ name: { type: 'string', required: true },
+ service_instance_guid: { type: 'string', required: true }
+ })
+ end
+ end
+
+ CREDENTIALS = { 'foo' => 'bar' }
+
+ let(:guid_pattern) { '[[:alnum:]-]+' }
+ let(:bind_status) { 200 }
+ let(:bind_body) { { credentials: CREDENTIALS } }
+ let(:unbind_status) { 200 }
+ let(:unbind_body) { {} }
+
+ def broker_url(broker)
+ base_broker_uri = URI.parse(broker.broker_url)
+ base_broker_uri.user = broker.auth_username
+ base_broker_uri.password = broker.auth_password
+ base_broker_uri.to_s
+ end
+
+ def stub_requests(broker)
+ stub_request(:put, %r{#{broker_url(broker)}/v2/service_instances/#{guid_pattern}/service_bindings/#{guid_pattern}}).
+ to_return(status: bind_status, body: bind_body.to_json)
+ end
+
+ def bind_url_regex(opts={})
+ service_binding = opts[:service_binding]
+ service_binding_guid = service_binding.try(:guid) || guid_pattern
+ service_instance = opts[:service_instance] || service_binding.try(:service_instance)
+ service_instance_guid = service_instance.try(:guid) || guid_pattern
+ broker = opts[:service_broker] || service_instance.service_plan.service.service_broker
+ %r{#{broker_url(broker)}/v2/service_instances/#{service_instance_guid}/service_bindings/#{service_binding_guid}}
+ end
+
+ describe 'create' do
+ context 'for managed instances' do
+ let(:instance) { ManagedServiceInstance.make }
+ let(:space) { instance.space }
+ let(:service) { instance.service }
+ let(:developer) { make_developer_for_space(space) }
+ let(:name) { 'fake-service-key' }
+ let(:service_instance_guid) { instance.guid }
+ let(:req) {
+ {
+ name: name,
+ service_instance_guid: service_instance_guid
+ }.to_json
+ }
+
+ before do
+ stub_requests(service.service_broker)
+ end
+
+ it 'creates a service key to a service instance' do
+ post '/v2/service_keys', req, json_headers(headers_for(developer))
+ expect(last_response).to have_status_code(201)
+ service_key = ServiceKey.last
+ expect(service_key.credentials).to eq(CREDENTIALS)
+ end
+
+ context 'when attempting to create service key for an unbindable service' do
+ before do
+ service.bindable = false
+ service.save
+
+ req = {
+ name: name,
+ service_instance_guid: instance.guid }.to_json
+
+ post '/v2/service_keys', req, json_headers(headers_for(developer))
+ end
+
+ it 'raises UnbindableService error' do
+ hash_body = JSON.parse(last_response.body)
+ expect(hash_body['error_code']).to eq('CF-UnbindableService')
+ expect(last_response).to have_status_code(400)
+ end
+
+ it 'does not send a bind request to broker' do
+ expect(a_request(:put, bind_url_regex(service_instance: instance))).to_not have_been_made
+ end
+ end
+
+ context 'when the service instance is invalid' do
+ context 'because service_instance_guid is invalid' do
+ let(:service_instance_guid) { 'THISISWRONG' }
+
+ it 'returns CF-ServiceInstanceNotFound error' do
+ post '/v2/service_keys', req, json_headers(headers_for(developer))
+
+ hash_body = JSON.parse(last_response.body)
+ expect(hash_body['error_code']).to eq('CF-ServiceInstanceNotFound')
+ expect(last_response.status).to eq(404)
+ end
+ end
+
+ context 'when the instance operation is in progress' do
+ before do
+ instance.save_with_operation(
+ last_operation: {
+ type: 'delete',
+ state: 'in progress',
+ }
+ )
+ end
+
+ it 'does not tell the service broker to bind the service' do
+ broker = service.service_broker
+ post '/v2/service_keys', req, json_headers(headers_for(developer))
+
+ expect(a_request(:put, %r{#{broker_url(broker)}/v2/service_instances/#{guid_pattern}/service_bindings/#{guid_pattern}})).
+ to_not have_been_made
+ end
+
+ it 'should show an error message for create key operation' do
+ post '/v2/service_keys', req, json_headers(headers_for(developer))
+ expect(last_response).to have_status_code 400
+ expect(last_response.body).to match 'ServiceInstanceOperationInProgress'
+ end
+ end
+
+ describe 'locking the instance as a result of creating service key' do
+ context 'when the instance has a previous operation' do
+ before do
+ instance.service_instance_operation = ServiceInstanceOperation.make(type: 'create', state: 'succeeded')
+ instance.save
+ end
+
+ it 'reverts the last_operation of the instance to its previous operation' do
+ post '/v2/service_keys', req, json_headers(headers_for(developer))
+ expect(instance.last_operation.state).to eq 'succeeded'
+ expect(instance.last_operation.type).to eq 'create'
+ end
+ end
+
+ context 'when the instance does not have a last_operation' do
+ before do
+ instance.service_instance_operation = nil
+ instance.save
+ end
+
+ it 'does not save a last_operation' do
+ post '/v2/service_keys', req, json_headers(headers_for(developer))
+ expect(instance.refresh.last_operation).to be_nil
+ end
+ end
+ end
+
+ describe 'creating key errors' do
+ subject(:make_request) do
+ post '/v2/service_keys', req, json_headers(headers_for(developer))
+ end
+
+ context 'when attempting to create key and service key already exists' do
+ before do
+ ServiceKey.make(name: name, service_instance: instance)
+ end
+
+ it 'returns a ServiceKeyTaken error' do
+ make_request
+ expect(last_response.status).to eq(400)
+ expect(decoded_response['error_code']).to eq('CF-ServiceKeyTaken')
+ end
+
+ it 'does not send a bind request to broker' do
+ make_request
+ expect(a_request(:put, bind_url_regex(service_instance: instance))).to_not have_been_made
+ end
+ end
+
+ context 'when the v2 broker returns a 409' do
+ let(:bind_status) { 409 }
+ let(:bind_body) { {} }
+
+ it 'returns a 409' do
+ make_request
+ expect(last_response).to have_status_code 409
+ end
+
+ it 'returns a ServiceBrokerConflict error' do
+ make_request
+ expect(decoded_response['error_code']).to eq 'CF-ServiceBrokerConflict'
+ end
+ end
+
+ context 'when the v2 broker returns any other error' do
+ let(:bind_status) { 500 }
+ let(:bind_body) { { description: 'ERROR MESSAGE HERE' } }
+
+ context 'when the instance has a last_operation' do
+ before do
+ instance.service_instance_operation = ServiceInstanceOperation.make(type: 'create', state: 'succeeded')
+ end
+
+ it 'rolls back the last_operation of the service instance' do
+ make_request
+ expect(instance.refresh.last_operation.state).to eq 'succeeded'
+ expect(instance.refresh.last_operation.type).to eq 'create'
+ end
+ end
+ end
+ end
+ end
+ end
+ end
+ end
+end
View
16 spec/unit/lib/services/service_brokers/v2/client_spec.rb
@@ -814,6 +814,12 @@ module VCAP::Services::ServiceBrokers::V2
app: app
)
end
+ let(:service_key) do
+ VCAP::CloudController::ServiceKey.new(
+ name: 'fake-service_key',
+ service_instance: instance
+ )
+ end
let(:response_data) do
{
@@ -853,6 +859,16 @@ module VCAP::Services::ServiceBrokers::V2
)
end
+ it 'makes a put request to create a service key with correct message' do
+ client.bind(service_key)
+
+ expect(http_client).to have_received(:put).
+ with(anything,
+ plan_id: binding.service_plan.broker_provided_id,
+ service_id: binding.service.broker_provided_id
+ )
+ end
+
it 'sets the credentials on the binding' do
attributes = client.bind(binding)
# ensure attributes return match ones for the database
View
79 spec/unit/models/services/service_key_spec.rb
@@ -0,0 +1,79 @@
+require 'spec_helper'
+
+module VCAP::CloudController
+ describe VCAP::CloudController::ServiceKey, type: :model do
+ let(:client) { double('broker client', unbind: nil, deprovision: nil) }
+
+ before do
+ allow_any_instance_of(Service).to receive(:client).and_return(client)
+ end
+
+ it { is_expected.to have_timestamp_columns }
+
+ describe 'Associations' do
+ it { is_expected.to have_associated :service_instance, associated_instance: ->(service_key) { ServiceInstance.make(space: service_key.space) } }
+ end
+
+ describe 'Validations' do
+ it { is_expected.to validate_presence :service_instance }
+ it { is_expected.to validate_presence :name }
+ it { is_expected.to validate_db_presence :service_instance_id }
+ it { is_expected.to validate_db_presence :credentials }
+ it { is_expected.to validate_uniqueness [:name, :service_instance_id] }
+ end
+
+ describe 'Serialization' do
+ it { is_expected.to export_attributes :name, :service_instance_guid, :credentials }
+ it { is_expected.to import_attributes :name, :service_instance_guid, :credentials }
+ end
+
+ describe '#new' do
+ it 'has a guid when constructed' do
+ service_key = described_class.new
+ expect(service_key.guid).to be
+ end
+ end
+
+ it_behaves_like 'a model with an encrypted attribute' do
+ let(:service_instance) { ManagedServiceInstance.make }
+
+ def new_model
+ ServiceKey.make(
+ name: Sham.name,
+ service_instance: service_instance,
+ credentials: value_to_encrypt
+ )
+ end
+
+ let(:encrypted_attr) { :credentials }
+ let(:attr_salt) { :salt }
+ end
+
+ describe '#to_hash' do
+ let(:service_key) { ServiceKey.make }
+ let(:developer) { make_developer_for_space(service_key.service_instance.space) }
+ let(:auditor) { make_auditor_for_space(service_key.service_instance.space) }
+ let(:user) { make_user_for_space(service_key.service_instance.space) }
+
+ it 'does not redact creds for an admin' do
+ allow(VCAP::CloudController::SecurityContext).to receive(:admin?).and_return(true)
+ expect(service_key.to_hash['credentials']).not_to eq({ redacted_message: '[PRIVATE DATA HIDDEN]' })
+ end
+
+ it 'does not redact creds for a space developer' do
+ allow(VCAP::CloudController::SecurityContext).to receive(:current_user).and_return(developer)
+ expect(service_key.to_hash['credentials']).not_to eq({ redacted_message: '[PRIVATE DATA HIDDEN]' })
+ end
+
+ it 'redacts creds for a space auditor' do
+ allow(VCAP::CloudController::SecurityContext).to receive(:current_user).and_return(auditor)
+ expect(service_key.to_hash['credentials']).to eq({ redacted_message: '[PRIVATE DATA HIDDEN]' })
+ end
+
+ it 'redacts creds for a space user' do
+ allow(VCAP::CloudController::SecurityContext).to receive(:current_user).and_return(user)
+ expect(service_key.to_hash['credentials']).to eq({ redacted_message: '[PRIVATE DATA HIDDEN]' })
+ end
+ end
+ end
+end
Please sign in to comment.
Something went wrong with that request. Please try again.