v3(services) Service key creation validation of the related SI

[#174145595](https://www.pivotaltracker.com/story/show/174145595)

Author: FelisiaM

app/actions/service_credential_binding_key_create.rb

@@ -0,0 +1,41 @@
+module VCAP::CloudController
+  module V3
+    class ServiceCredentialBindingKeyCreate
+      class UnprocessableCreate < StandardError
+      end
+
+      def precursor(service_instance, volume_mount_services_enabled: false)
+        if service_instance.managed_instance?
+          service_not_bindable! unless service_instance.service_plan.bindable?
+          service_not_available! unless service_instance.service_plan.active?
+          volume_mount_not_enabled! if service_instance.volume_service? && !volume_mount_services_enabled
+          operation_in_progress! if service_instance.operation_in_progress?
+        else
+          key_not_supported_for_user_provided_service!
+        end
+      end
+
+      private
+
+      def key_not_supported_for_user_provided_service!
+        raise UnprocessableCreate.new("Service credential bindings of type 'key' are not supported for user-provided service instances.")
+      end
+
+      def operation_in_progress!
+        raise UnprocessableCreate.new('There is an operation in progress for the service instance.')
+      end
+
+      def service_not_bindable!
+        raise UnprocessableCreate.new('Service plan does not allow bindings.')
+      end
+
+      def service_not_available!
+        raise UnprocessableCreate.new('Service plan is not available.')
+      end
+
+      def volume_mount_not_enabled!
+        raise UnprocessableCreate.new('Support for volume mount services is disabled.')
+      end
+    end
+  end
+end

app/controllers/v3/service_credential_bindings_controller.rb

@@ -1,4 +1,5 @@
 require 'actions/service_credential_binding_create'
+require 'actions/service_credential_binding_key_create'
 require 'actions/service_credential_binding_delete'
 require 'fetchers/service_credential_binding_fetcher'
 require 'fetchers/service_credential_binding_list_fetcher'
@@ -44,12 +45,14 @@ def show
   def create
     message = build_create_message(hashed_params[:body])
 
-    if message.type == 'app'
-      service_instance = VCAP::CloudController::ServiceInstance.first(guid: message.service_instance_guid)
-      resource_not_accessible!('service instance', message.service_instance_guid) unless can_read_service_instance?(service_instance)
+    service_instance = VCAP::CloudController::ServiceInstance.first(guid: message.service_instance_guid)
+    resource_not_accessible!('service instance', message.service_instance_guid) unless can_read_service_instance?(service_instance)
 
+    case message.type
+    when 'app'
       app = VCAP::CloudController::AppModel.first(guid: message.app_guid)
       resource_not_accessible!('app', message.app_guid) unless can_access_resource?(app)
+
       unauthorized! unless can_write_to_space?(app.space)
 
       action = V3::ServiceCredentialBindingCreate.new(user_audit_info, message.audit_hash)
@@ -63,11 +66,19 @@ def create
         action.bind(binding)
         render status: :created, json: Presenters::V3::ServiceCredentialBindingPresenter.new(binding).to_hash
       end
-    else
+    when 'key'
+      unauthorized! unless can_write_to_space?(service_instance.space)
+
+      V3::ServiceCredentialBindingKeyCreate.new.precursor(
+        service_instance,
+        volume_mount_services_enabled: volume_services_enabled?
+      )
+
       head :not_implemented
       return
     end
-  rescue V3::ServiceCredentialBindingCreate::UnprocessableCreate => e
+  rescue V3::ServiceCredentialBindingCreate::UnprocessableCreate,
+    V3::ServiceCredentialBindingKeyCreate::UnprocessableCreate => e
     unprocessable!(e.message)
   end
 
@@ -284,6 +295,7 @@ def operation_in_progress!
     unprocessable!('There is an operation in progress for the service instance.')
   end
 
+
   def not_found!
     resource_not_found!(:service_credential_binding)
   end

spec/request/service_credential_bindings_spec.rb

@@ -836,7 +836,6 @@ def check_filtered_bindings(*bindings)
         credentials: { password: 'foo' }
       }
     }
-    let(:service_instance) { VCAP::CloudController::UserProvidedServiceInstance.make(space: space, **service_instance_details) }
     let(:service_instance_guid) { service_instance.guid }
 
     context 'creating a credential binding to an app' do
@@ -983,6 +982,8 @@ def check_filtered_bindings(*bindings)
       end
 
       context 'user-provided service' do
+        let(:service_instance) { VCAP::CloudController::UserProvidedServiceInstance.make(space: space, **service_instance_details) }
+
         it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS do
           let(:expected_codes_and_responses) do
             Hash.new(code: 403).tap do |h|
@@ -1472,6 +1473,7 @@ def check_filtered_bindings(*bindings)
     end
 
     context 'creating a credential binding as a key' do
+      let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space: space, **service_instance_details) }
       let(:create_body) {
         {
           type: 'key',
@@ -1482,16 +1484,148 @@ def check_filtered_bindings(*bindings)
         }.merge(request_extra)
       }
 
-      it 'returns 422 when type is missing' do
-        create_body.delete(:type)
+      context 'permissions' do
+        context 'users in the originating service instance space' do
+          it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS do
+            let(:expected_codes_and_responses) do
+              Hash.new(code: 403).tap do |h|
+                h['space_developer'] = { code: 501 }
+                h['admin'] = { code: 501 }
+                h['org_billing_manager'] = { code: 422 }
+                h['org_auditor'] = { code: 422 }
+                h['no_role'] = { code: 422 }
+              end
+            end
+          end
+        end
 
-        api_call.call admin_headers
-        expect(last_response).to have_status_code(422)
-        expect(parsed_response['errors']).to include(include({
-          'detail' => include("Type must be 'app' or 'key'"),
-          'title' => 'CF-UnprocessableEntity',
-          'code' => 10008,
-        }))
+        context 'users in the space where the SI has been shared to' do
+          let(:orginal_org) { VCAP::CloudController::Organization.make }
+          let(:original_space) { VCAP::CloudController::Space.make(organization: orginal_org) }
+          let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space: original_space) }
+
+          before do
+            service_instance.add_shared_space(space)
+          end
+
+          it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS do
+            let(:expected_codes_and_responses) do
+              Hash.new(code: 403).tap do |h|
+                h['admin'] = { code: 501 }
+                h['org_billing_manager'] = { code: 422 }
+                h['org_auditor'] = { code: 422 }
+                h['no_role'] = { code: 422 }
+              end
+            end
+          end
+        end
+      end
+
+      context 'request validation' do
+        it 'returns 422 when type is missing' do
+          create_body.delete(:type)
+
+          api_call.call admin_headers
+          expect(last_response).to have_status_code(422)
+          expect(parsed_response['errors']).to include(include({
+            'detail' => include("Type must be 'app' or 'key'"),
+            'title' => 'CF-UnprocessableEntity',
+            'code' => 10008,
+          }))
+        end
+
+        it 'returns 422 when service instance relationship is not included' do
+          create_body[:relationships].delete(:service_instance)
+          api_call.call admin_headers
+          expect(last_response).to have_status_code(422)
+          expect(parsed_response['errors']).to include(include({
+            'detail' => include("Relationships 'relationships' must include one or more valid relationships"),
+            'title' => 'CF-UnprocessableEntity',
+            'code' => 10008,
+          }))
+          end
+
+        context 'when the service instance does not exist' do
+          let(:service_instance_guid) { 'fake-instance' }
+
+          it 'returns a 422 when the service instance does not exist' do
+            api_call.call admin_headers
+
+            expect(last_response).to have_status_code(422)
+            expect(parsed_response['errors']).to include(include({
+              'detail' => include("The service instance could not be found: 'fake-instance'"),
+              'title' => 'CF-UnprocessableEntity',
+              'code' => 10008,
+            }))
+          end
+        end
+
+        context 'when the user has no access to the service instance' do
+          let(:space_user) do
+            u = VCAP::CloudController::User.make
+            other_space.organization.add_user(u)
+            other_space.add_developer(u)
+            u
+          end
+          let(:space_dev_headers) { headers_for(space_user) }
+
+          it 'returns a 422' do
+              api_call.call space_dev_headers
+              expect(last_response).to have_status_code(422)
+              expect(parsed_response['errors']).to include(include({
+                'detail' => include("The service instance could not be found: '#{service_instance_guid}'"),
+                'title' => 'CF-UnprocessableEntity',
+                'code' => 10008,
+              }))
+            end
+        end
+
+        context 'when the service instance is user-provided' do
+          let(:service_instance) { VCAP::CloudController::UserProvidedServiceInstance.make(space: space, **service_instance_details) }
+
+          it 'returns a 422' do
+            api_call.call admin_headers
+
+            expect(last_response).to have_status_code(422)
+            expect(parsed_response['errors']).to include(include({
+              'detail' => include("Service credential bindings of type 'key' are not supported for user-provided service instances."),
+              'title' => 'CF-UnprocessableEntity',
+              'code' => 10008,
+            }))
+          end
+        end
+
+        context 'when the service instance is not bindable' do
+          let(:plan) { VCAP::CloudController::ServicePlan.make(bindable: false) }
+          let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space: space, service_plan: plan) }
+
+          it 'returns a 422' do
+            api_call.call admin_headers
+
+            expect(last_response).to have_status_code(422)
+            expect(parsed_response['errors']).to include(include({
+              'detail' => include("Service plan does not allow bindings."),
+              'title' => 'CF-UnprocessableEntity',
+              'code' => 10008,
+            }))
+          end
+        end
+
+        context 'when the service instance is from unavailable plan' do
+          let(:plan) { VCAP::CloudController::ServicePlan.make(active: false) }
+          let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space: space, service_plan: plan) }
+
+          it 'returns a 422' do
+            api_call.call admin_headers
+
+            expect(last_response).to have_status_code(422)
+            expect(parsed_response['errors']).to include(include({
+              'detail' => include("Service plan is not available."),
+              'title' => 'CF-UnprocessableEntity',
+              'code' => 10008,
+            }))
+          end
+        end
       end
 
       it 'should return 501' do