Skip to content

Commit

Permalink
Ensure space supporter receives error for specific manifest endpoints (
Browse files Browse the repository at this point in the history
…#2398)

Co-authored-by: Matthew Kocher <mkocher@pivotal.io>
Co-authored-by: Carson Long <lcarson@vmware.com>
  • Loading branch information
mkocher and ctlong committed Jul 15, 2021
1 parent fddf001 commit 664d607
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 4 deletions.
2 changes: 1 addition & 1 deletion app/controllers/v3/app_manifests_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class AppManifestsController < ApplicationController
def show
app, space, org = AppFetcher.new.fetch(hashed_params[:guid])

app_not_found! unless app && permission_queryer.can_read_from_space?(space.guid, org.guid)
app_not_found! unless app && permission_queryer.untrusted_can_read_from_space?(space.guid, org.guid)
unauthorized! unless permission_queryer.can_read_secrets_in_space?(space.guid, org.guid)

manifest_presenter = Presenters::V3::AppManifestPresenter.new(app, app.service_bindings, app.routes)
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/v3/space_manifests_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class SpaceManifestsController < ApplicationController

def apply_manifest
space = Space.find(guid: hashed_params[:guid])
space_not_found! unless space && permission_queryer.can_read_from_space?(space.guid, space.organization.guid)
space_not_found! unless space && permission_queryer.untrusted_can_read_from_space?(space.guid, space.organization.guid)
unauthorized! unless permission_queryer.can_write_to_space?(space.guid)

messages = parsed_app_manifests.map { |app_manifest| AppManifestMessage.create_from_yml(app_manifest) }
Expand Down Expand Up @@ -44,7 +44,7 @@ def apply_manifest

def diff_manifest
space = Space.find(guid: hashed_params[:guid])
space_not_found! unless space && permission_queryer.can_read_from_space?(space.guid, space.organization.guid)
space_not_found! unless space && permission_queryer.untrusted_can_read_from_space?(space.guid, space.organization.guid)
unauthorized! unless permission_queryer.can_write_to_space?(space.guid)

parsed_manifests = parsed_app_manifests.map(&:to_hash)
Expand Down
24 changes: 24 additions & 0 deletions spec/request/app_manifests_spec.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
require 'spec_helper'
require 'request_spec_shared_examples'

RSpec.describe 'App Manifests' do
let(:user) { VCAP::CloudController::User.make }
let(:user_header) { headers_for(user, email: Sham.email, user_name: 'some-username') }
let(:space) { VCAP::CloudController::Space.make }
let(:org) { space.organization }
let(:shared_domain) { VCAP::CloudController::SharedDomain.make }
let(:route) { VCAP::CloudController::Route.make(domain: shared_domain, space: space, host: 'a_host') }
let(:second_route) {
Expand Down Expand Up @@ -50,6 +52,28 @@
let!(:sidecar_process_type2) { VCAP::CloudController::SidecarProcessTypeModel.make(type: 'web', sidecar: sidecar1, app_guid: app_model.guid) }
let!(:sidecar_process_type3) { VCAP::CloudController::SidecarProcessTypeModel.make(type: 'other_worker', sidecar: sidecar2, app_guid: app_model.guid) }

context 'permissions' do
let(:api_call) { lambda { |user_headers| get "/v3/apps/#{app_model.guid}/manifest", nil, user_headers } }
let(:expected_codes_and_responses) do
h = Hash.new(code: 403)
h['no_role'] = { code: 404 }
h['org_auditor'] = { code: 404 }
h['org_billing_manager'] = { code: 404 }

h['admin'] = { code: 200 }
h['admin_read_only'] = { code: 200 }
h['space_developer'] = { code: 200 }

h
end

before do
space.remove_developer(user)
end

it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + ['space_supporter']
end

context 'for a buildpack' do
let!(:buildpack) { VCAP::CloudController::Buildpack.make }
let!(:buildpack2) { VCAP::CloudController::Buildpack.make }
Expand Down
24 changes: 23 additions & 1 deletion spec/request/space_manifests_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,28 @@
'berry' => 'white', }, VCAP::CloudController::AppAnnotationModel)
end

context 'permissions' do
before do
space.remove_developer(user)
end

it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS + ['space_supporter'] do
let(:api_call) { lambda { |user_headers| post "/v3/spaces/#{space.guid}/actions/apply_manifest", yml_manifest, yml_headers(user_headers) } }
let(:org) { space.organization }
let(:expected_codes_and_responses) do
h = Hash.new(code: 403)
h['org_auditor'] = { code: 404 }
h['org_billing_manager'] = { code: 404 }
h['no_role'] = { code: 404 }

h['admin'] = { code: 202 }
h['space_developer'] = { code: 202 }

h.freeze
end
end
end

it 'applies the manifest' do
web_process = app1_model.web_processes.first
expect(web_process.instances).to eq(1)
Expand Down Expand Up @@ -628,7 +650,7 @@
default_manifest.to_yaml
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 'when the app name has changed' do
Expand Down

0 comments on commit 664d607

Please sign in to comment.