Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow space application supporter to access specific service plan endpoints #2349

Merged
merged 6 commits into from
Jul 15, 2021
Merged

Allow space application supporter to access specific service plan endpoints #2349

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e8e420f
Allow space application supporter to access specific service plan end…
svkrieger Jun 22, 2021
fc57d06
Refactor permissions
svkrieger Jun 29, 2021
4d2cccc
swap COMPLETE_PERMISSIONS with ALL_PERMISSIONS
iaftab-alam Jul 1, 2021
b44f894
Rename space application supporter to space supporter
svkrieger Jul 15, 2021
cf23456
Fix method name
svkrieger Jul 15, 2021
21aecc9
Rename leftovers after rebase to main
svkrieger Jul 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/controllers/v3/mixins/service_permissions.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/v3/service_plans_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
8 changes: 8 additions & 0 deletions lib/cloud_controller/permissions.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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?

Expand Down
29 changes: 15 additions & 14 deletions spec/request/service_plan_visibility_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -169,21 +170,21 @@
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
let(:req_body) { { type: 'organization', organizations: [{ guid: org.guid }, { guid: other_org.guid }] } }
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

Expand All @@ -199,22 +200,22 @@
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
let(:req_body) { { type: 'organization', organizations: [{ guid: org.guid }, { guid: other_org.guid }] } }
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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
33 changes: 18 additions & 15 deletions spec/request/service_plans_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down