From dc4d9fda576af8d4c0d4bc456cfa169137184625 Mon Sep 17 00:00:00 2001
From: Sven Krieger <37476281+svkrieger@users.noreply.github.com>
Date: Thu, 15 Jul 2021 23:28:09 +0200
Subject: [PATCH] Allow space application supporter to access specific service
 plan endpoints (#2349)

* Allow space application supporter to access specific service plan endpoints

Signed-off-by: Aftab Alam <81828613+iaftab-alam@users.noreply.github.com>
---
 .../v3/mixins/service_permissions.rb          |  2 +-
 .../v3/service_plans_controller.rb            |  2 +-
 lib/cloud_controller/permissions.rb           |  8 +++++
 spec/request/service_plan_visibility_spec.rb  | 29 ++++++++--------
 spec/request/service_plans_spec.rb            | 33 ++++++++++---------
 5 files changed, 43 insertions(+), 31 deletions(-)

diff --git a/app/controllers/v3/mixins/service_permissions.rb b/app/controllers/v3/mixins/service_permissions.rb
index 968696f1c7b..caa0bd07be6 100644
--- a/app/controllers/v3/mixins/service_permissions.rb
+++ b/app/controllers/v3/mixins/service_permissions.rb
@@ -26,7 +26,7 @@ def current_user_can_write?(resource)
     end
 
     def visible_space_scoped?(space)
-      current_user && space && space.has_member?(current_user)
+      current_user && space && (space.has_member?(current_user) || space.has_supporter?(current_user))
     end
 
     def visible_to_current_user?(service: nil, plan: nil)
diff --git a/app/controllers/v3/service_plans_controller.rb b/app/controllers/v3/service_plans_controller.rb
index 63c98316b0e..0802280a7e8 100644
--- a/app/controllers/v3/service_plans_controller.rb
+++ b/app/controllers/v3/service_plans_controller.rb
@@ -55,7 +55,7 @@ def index
                   message,
                   eager_loaded_associations: Presenters::V3::ServicePlanPresenter.associated_resources,
                   readable_org_guids: permission_queryer.readable_org_guids,
-                  readable_space_guids: permission_queryer.readable_space_scoped_space_guids,
+                  readable_space_guids: permission_queryer.readable_space_supporter_space_scoped_space_guids,
                 )
               end
 
diff --git a/lib/cloud_controller/permissions.rb b/lib/cloud_controller/permissions.rb
index 3c1bc1280c7..874492fe90f 100644
--- a/lib/cloud_controller/permissions.rb
+++ b/lib/cloud_controller/permissions.rb
@@ -234,6 +234,14 @@ def readable_space_scoped_space_guids
     end
   end
 
+  def readable_space_supporter_space_scoped_space_guids
+    if can_read_globally?
+      VCAP::CloudController::Space.select(:guid).all.map(&:guid)
+    else
+      membership.space_guids_for_roles(SPACE_ROLES_INCLUDING_SUPPORTERS)
+    end
+  end
+
   def can_read_route?(space_guid, org_guid)
     return true if can_read_globally?
 
diff --git a/spec/request/service_plan_visibility_spec.rb b/spec/request/service_plan_visibility_spec.rb
index 6bdef39506b..82d9599578e 100644
--- a/spec/request/service_plan_visibility_spec.rb
+++ b/spec/request/service_plan_visibility_spec.rb
@@ -31,7 +31,7 @@
         )
       }
 
-      it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS
+      it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + ['space_supporter']
     end
 
     context 'for admin-only plans' do
@@ -50,7 +50,7 @@
         end
       }
 
-      it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS
+      it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + ['space_supporter']
     end
 
     context 'for space-scoped plans' do
@@ -87,11 +87,12 @@
             space_developer
             space_manager
             space_auditor
+            space_supporter
           )
         )
       end
 
-      it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS
+      it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + ['space_supporter']
     end
 
     context 'for org-restricted plans' do
@@ -142,7 +143,7 @@
         end
       }
 
-      it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS
+      it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + ['space_supporter']
     end
   end
 
@@ -169,13 +170,13 @@
         let(:req_body) { { type: 'admin' } }
         let(:successful_response) { { code: 200, response_object: { type: 'admin' } } }
 
-        it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS
+        it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + ['space_supporter']
       end
 
       context 'and its being updated to "public"' do
         let(:successful_response) { { code: 200, response_object: { type: 'public' } } }
 
-        it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS
+        it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + ['space_supporter']
       end
 
       context 'and its being updated to "organization"' do
@@ -183,7 +184,7 @@
         let(:org_response) { [{ name: org.name, guid: org.guid }, { name: other_org.name, guid: other_org.guid }] }
         let(:successful_response) { { code: 200, response_object: { type: 'organization', organizations: org_response } } }
 
-        it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS
+        it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + ['space_supporter']
       end
     end
 
@@ -199,14 +200,14 @@
       context 'and its being updated to "public"' do
         let(:successful_response) { { code: 200, response_object: { type: 'public' } } }
 
-        it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS
+        it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + ['space_supporter']
       end
 
       context 'and its being updated to "admin"' do
         let(:req_body) { { type: 'admin' } }
         let(:successful_response) { { code: 200, response_object: { type: 'admin' } } }
 
-        it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS
+        it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + ['space_supporter']
       end
 
       context 'and its being updated to "organization"' do
@@ -214,7 +215,7 @@
         let(:org_response) { [{ name: org.name, guid: org.guid }, { name: other_org.name, guid: other_org.guid }] }
         let(:successful_response) { { code: 200, response_object: { type: 'organization', organizations: org_response } } }
 
-        it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS
+        it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + ['space_supporter']
       end
     end
 
@@ -258,7 +259,7 @@
           end
         end
 
-        it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS do
+        it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + ['space_supporter'] do
           let(:after_request_check) do
             lambda do
               visibilities = VCAP::CloudController::ServicePlanVisibility.where(service_plan: service_plan).all
@@ -288,7 +289,7 @@
           end
         end
 
-        it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS do
+        it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + ['space_supporter'] do
           let(:after_request_check) do
             lambda do
               visibilities = VCAP::CloudController::ServicePlanVisibility.where(service_plan: service_plan).all
@@ -550,7 +551,7 @@
         end
       end
 
-      it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS
+      it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + ['space_supporter']
 
       it 'returns a 404 for users of other orgs' do
         new_org = VCAP::CloudController::Organization.make
@@ -629,7 +630,7 @@
         end
       end
 
-      it_behaves_like 'permissions for delete endpoint', ALL_PERMISSIONS
+      it_behaves_like 'permissions for delete endpoint', ALL_PERMISSIONS + ['space_supporter']
     end
 
     it 'creates an audit event' do
diff --git a/spec/request/service_plans_spec.rb b/spec/request/service_plans_spec.rb
index ab66544f514..7c9ff7443f2 100644
--- a/spec/request/service_plans_spec.rb
+++ b/spec/request/service_plans_spec.rb
@@ -4,7 +4,6 @@
 require 'hashdiff'
 
 UNAUTHENTICATED = %w[unauthenticated].freeze
-COMPLETE_PERMISSIONS = (ALL_PERMISSIONS + UNAUTHENTICATED).freeze
 
 RSpec.describe 'V3 service plans' do
   let(:user) { VCAP::CloudController::User.make }
@@ -21,7 +20,7 @@
         Hash.new(code: 404)
       end
 
-      it_behaves_like 'permissions for single object endpoint', COMPLETE_PERMISSIONS
+      it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + UNAUTHENTICATED + ['space_supporter']
     end
 
     context 'when there is a public service plan' do
@@ -41,7 +40,7 @@
         )
       end
 
-      it_behaves_like 'permissions for single object endpoint', COMPLETE_PERMISSIONS
+      it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + UNAUTHENTICATED + ['space_supporter']
 
       context 'when the hide_marketplace_from_unauthenticated_users feature flag is enabled' do
         before do
@@ -69,7 +68,7 @@
           end
         end
 
-        it_behaves_like 'permissions for single object endpoint', COMPLETE_PERMISSIONS
+        it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + UNAUTHENTICATED + ['space_supporter']
       end
 
       context 'space scoped broker' do
@@ -88,11 +87,12 @@
               space_developer
               space_manager
               space_auditor
+              space_supporter
             )
           )
         end
 
-        it_behaves_like 'permissions for single object endpoint', COMPLETE_PERMISSIONS
+        it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + UNAUTHENTICATED + ['space_supporter']
       end
     end
 
@@ -274,10 +274,11 @@
           h['space_developer'] = space_plans_response
           h['space_manager'] = space_plans_response
           h['space_auditor'] = space_plans_response
+          h['space_supporter'] = space_plans_response
         end
       end
 
-      it_behaves_like 'permissions for list endpoint', COMPLETE_PERMISSIONS
+      it_behaves_like 'permissions for list endpoint', ALL_PERMISSIONS + UNAUTHENTICATED + ['space_supporter']
 
       context 'when the hide_marketplace_from_unauthenticated_users feature flag is enabled' do
         before do
@@ -641,7 +642,7 @@
         end
       end
 
-      it_behaves_like 'permissions for delete endpoint', COMPLETE_PERMISSIONS
+      it_behaves_like 'permissions for delete endpoint', ALL_PERMISSIONS + UNAUTHENTICATED + ['space_supporter']
     end
 
     context 'when the service plan exists and has no service instances' do
@@ -659,7 +660,7 @@
           end
         end
 
-        it_behaves_like 'permissions for delete endpoint', COMPLETE_PERMISSIONS
+        it_behaves_like 'permissions for delete endpoint', ALL_PERMISSIONS + UNAUTHENTICATED + ['space_supporter']
       end
 
       context 'when the plan is public' do
@@ -672,7 +673,7 @@
           end
         end
 
-        it_behaves_like 'permissions for delete endpoint', COMPLETE_PERMISSIONS
+        it_behaves_like 'permissions for delete endpoint', ALL_PERMISSIONS + UNAUTHENTICATED + ['space_supporter']
       end
 
       context 'when the plan is visible only on some orgs' do
@@ -690,7 +691,7 @@
           end
         end
 
-        it_behaves_like 'permissions for delete endpoint', COMPLETE_PERMISSIONS
+        it_behaves_like 'permissions for delete endpoint', ALL_PERMISSIONS + UNAUTHENTICATED + ['space_supporter']
       end
 
       context 'when the plan is from a space-scoped service broker' do
@@ -704,13 +705,14 @@
             h['admin_read_only'] = { code: 403 }
             h['global_auditor'] = { code: 403 }
             h['space_developer'] = { code: 204 }
+            h['space_supporter'] = { code: 403 }
             h['space_manager'] = { code: 403 }
             h['space_auditor'] = { code: 403 }
             h['unauthenticated'] = { code: 401 }
           end
         end
 
-        it_behaves_like 'permissions for delete endpoint', COMPLETE_PERMISSIONS
+        it_behaves_like 'permissions for delete endpoint', ALL_PERMISSIONS + UNAUTHENTICATED + ['space_supporter']
       end
     end
 
@@ -809,7 +811,7 @@
           end
         end
 
-        it_behaves_like 'permissions for single object endpoint', COMPLETE_PERMISSIONS
+        it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + UNAUTHENTICATED + ['space_supporter']
       end
 
       context 'when the plan is public' do
@@ -822,7 +824,7 @@
           end
         end
 
-        it_behaves_like 'permissions for single object endpoint', COMPLETE_PERMISSIONS
+        it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + UNAUTHENTICATED + ['space_supporter']
       end
 
       context 'when the plan is visible only on some orgs' do
@@ -840,7 +842,7 @@
           end
         end
 
-        it_behaves_like 'permissions for single object endpoint', COMPLETE_PERMISSIONS
+        it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + UNAUTHENTICATED + ['space_supporter']
       end
 
       context 'when the plan is from a space-scoped service broker' do
@@ -854,13 +856,14 @@
             h['admin_read_only'] = { code: 403 }
             h['global_auditor'] = { code: 403 }
             h['space_developer'] = { code: 200, response_object: create_plan_json(service_plan, labels: labels, annotations: annotations) }
+            h['space_supporter'] = { code: 403 }
             h['space_manager'] = { code: 403 }
             h['space_auditor'] = { code: 403 }
             h['unauthenticated'] = { code: 401 }
           end
         end
 
-        it_behaves_like 'permissions for single object endpoint', COMPLETE_PERMISSIONS
+        it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + UNAUTHENTICATED + ['space_supporter']
       end
     end
   end