v3(services): Allow bind from shared spaces (cloudfoundry#1948)

* v3(services): Allow bind from shared spaces

- Updated permission on bind to make sure users in shared spaces can bind to apps
- Added tests for bind and unbind from shared spaces
[#175641070](https://www.pivotaltracker.com/story/show/175641070)

* Fixed indentation

Author: FelisiaM

app/controllers/v3/service_credential_bindings_controller.rb

@@ -44,7 +44,7 @@ def create
     unprocessable!(message.errors.full_messages) unless message.valid?
 
     service_instance = VCAP::CloudController::ServiceInstance.first(guid: message.service_instance_guid)
-    resource_not_accessible!('service instance', message.service_instance_guid) unless can_access_resource?(service_instance)
+    resource_not_accessible!('service instance', message.service_instance_guid) unless can_read_service_instance?(service_instance)
 
     app = VCAP::CloudController::AppModel.first(guid: message.app_guid)
     resource_not_accessible!('app', message.app_guid) unless can_access_resource?(app)
@@ -152,6 +152,16 @@ def can_access_resource?(resource)
     resource.present? && can_read_from_space?(resource.space)
   end
 
+  def can_read_service_instance?(service_instance)
+    if service_instance.present?
+      readable_spaces = service_instance.shared_spaces + [service_instance.space]
+
+      readable_spaces.any? do |space|
+        permission_queryer.can_read_from_space?(space.guid, space.organization_guid)
+      end
+    end
+  end
+
   def can_write_to_space?(space)
     permission_queryer.can_write_to_space?(space.guid)
   end

spec/request/service_credential_bindings_spec.rb

@@ -1058,14 +1058,40 @@ def check_filtered_bindings(*bindings)
       let(:audit) { VCAP::CloudController::Event.last }
       let(:job) { VCAP::CloudController::PollableJobModel.last }
 
-      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: 202 }
-            h['admin'] = { code: 202 }
-            h['org_billing_manager'] = { code: 422 }
-            h['org_auditor'] = { code: 422 }
-            h['no_role'] = { code: 422 }
+      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: 202 }
+                h['admin'] = { code: 202 }
+                h['org_billing_manager'] = { code: 422 }
+                h['org_auditor'] = { code: 422 }
+                h['no_role'] = { code: 422 }
+              end
+            end
+          end
+        end
+
+        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, service_plan: plan) }
+
+          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['space_developer'] = { code: 202 }
+                h['admin'] = { code: 202 }
+                h['org_billing_manager'] = { code: 422 }
+                h['org_auditor'] = { code: 422 }
+                h['no_role'] = { code: 422 }
+              end
+            end
           end
         end
       end
@@ -1452,6 +1478,24 @@ def check_filtered_bindings(*bindings)
     let(:service_instance) { VCAP::CloudController::UserProvidedServiceInstance.make(**service_instance_details) }
 
     context 'user provided services' do
+      context 'permissions' do
+        let(:db_check) {
+          lambda {
+            get "/v3/service_credential_bindings/#{guid}", {}, admin_headers
+            expect(last_response).to have_status_code(404)
+          }
+        }
+
+        let(:expected_codes_and_responses) do
+          Hash.new(code: 403).tap do |h|
+            h['admin'] = h['space_developer'] = { code: 204 }
+            h['org_billing_manager'] = h['org_auditor'] = h['no_role'] = { code: 404 }
+          end
+        end
+
+        it_behaves_like 'permissions for delete endpoint', ALL_PERMISSIONS
+      end
+
       context 'app bindings' do
         it 'can successfully delete the record' do
           api_call.call(admin_headers)
@@ -1499,6 +1543,44 @@ def check_filtered_bindings(*bindings)
         }
       end
 
+      context 'permissions' do
+        let(:db_check) {
+          lambda {
+            expect(last_response.headers['Location']).to match(%r(http.+/v3/jobs/[a-fA-F0-9-]+))
+          }
+        }
+
+        context 'users in the originating service instance space' do
+          it_behaves_like 'permissions for delete endpoint', ALL_PERMISSIONS do
+            let(:expected_codes_and_responses) do
+              Hash.new(code: 403).tap do |h|
+                h['admin'] = h['space_developer'] = { code: 202 }
+                h['org_billing_manager'] = h['org_auditor'] = h['no_role'] = { code: 404 }
+              end
+            end
+          end
+        end
+
+        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 delete endpoint', ALL_PERMISSIONS do
+            let(:expected_codes_and_responses) do
+              Hash.new(code: 403).tap do |h|
+                h['admin'] = h['space_developer'] = { code: 202 }
+                h['org_billing_manager'] = h['org_auditor'] = h['no_role'] = { code: 404 }
+              end
+            end
+          end
+        end
+      end
+
       context 'app binding' do
         it 'responds with a job resource' do
           api_call.call(space_dev_headers)
@@ -1788,24 +1870,6 @@ def check_filtered_bindings(*bindings)
       end
     end
 
-    context 'permissions' do
-      let(:db_check) {
-        lambda {
-          get "/v3/service_credential_bindings/#{guid}", {}, admin_headers
-          expect(last_response).to have_status_code(404)
-        }
-      }
-
-      let(:expected_codes_and_responses) do
-        Hash.new(code: 403).tap do |h|
-          h['admin'] = h['space_developer'] = { code: 204 }
-          h['org_billing_manager'] = h['org_auditor'] = h['no_role'] = { code: 404 }
-        end
-      end
-
-      it_behaves_like 'permissions for delete endpoint', ALL_PERMISSIONS
-    end
-
     describe 'invalid requests' do
       context 'when the binding does not exist' do
         let(:guid) { 'fake-binding' }