diff --git a/app/access/organization_access.rb b/app/access/organization_access.rb index 4be64f39cc5..b05411a8148 100644 --- a/app/access/organization_access.rb +++ b/app/access/organization_access.rb @@ -1,7 +1,8 @@ module VCAP::CloudController class OrganizationAccess < BaseAccess - def create?(_org, _params=nil) + def create?(_org, params=nil) return true if context.queryer.can_write_globally? + return false if params.present? && params[:status.to_s] == VCAP::CloudController::Organization::SUSPENDED FeatureFlag.enabled?(:user_org_creation) end @@ -12,6 +13,7 @@ def read_for_update?(org, params=nil) return false unless context.queryer.can_write_to_active_org?(org.id) return false if params.present? && (params.key?(:quota_definition_guid.to_s) || params.key?(:billing_enabled.to_s)) + return false if params.present? && params[:status.to_s] == VCAP::CloudController::Organization::SUSPENDED true end diff --git a/app/controllers/v3/organizations_controller.rb b/app/controllers/v3/organizations_controller.rb index f4051deddee..e39947b5949 100644 --- a/app/controllers/v3/organizations_controller.rb +++ b/app/controllers/v3/organizations_controller.rb @@ -50,6 +50,9 @@ def create message = VCAP::CloudController::OrganizationCreateMessage.new(hashed_params[:body]) unprocessable!(message.errors.full_messages) unless message.valid? + + unauthorized! if message.requested?(:suspended) && message.suspended && !permission_queryer.can_write_globally? + org = nil Organization.db.transaction do @@ -78,6 +81,8 @@ def update message = VCAP::CloudController::OrganizationUpdateMessage.new(hashed_params[:body]) unprocessable!(message.errors.full_messages) unless message.valid? + unauthorized! if message.requested?(:suspended) && message.suspended && !permission_queryer.can_write_globally? + org = OrganizationUpdate.new(user_audit_info).update(org, message) render json: Presenters::V3::OrganizationPresenter.new(org), status: :ok diff --git a/docs/v3/source/includes/resources/organizations/_create.md.erb b/docs/v3/source/includes/resources/organizations/_create.md.erb index ce161181e78..ecfd933f6b3 100644 --- a/docs/v3/source/includes/resources/organizations/_create.md.erb +++ b/docs/v3/source/includes/resources/organizations/_create.md.erb @@ -42,8 +42,8 @@ Name | Type | Description **metadata.annotations** | [_annotation object_](#annotations) | Annotations applied to the organization #### Permitted roles - | ---- | --- -Admin | -If the `user_org_creation` feature flag is enabled, any user with the `cloud_controller.write` scope will be able to create organizations. +Role | Notes +---- | ----- +Admin | +All Roles | If the `user_org_creation` feature flag is enabled, any user with the `cloud_controller.write` scope can create organizations, but cannot set the `suspended` field to `true` diff --git a/docs/v3/source/includes/resources/organizations/_update.md.erb b/docs/v3/source/includes/resources/organizations/_update.md.erb index 6a3d2ff062d..9e6f08cf059 100644 --- a/docs/v3/source/includes/resources/organizations/_update.md.erb +++ b/docs/v3/source/includes/resources/organizations/_update.md.erb @@ -37,7 +37,8 @@ Name | Type | Description **metadata.annotations** | [_annotation object_](#annotations) | Annotations applied to the organization #### Permitted roles - | ---- | --- + +Role | Notes +---- | ----- Admin | -Org Manager | +Org Manager | Cannot change the `suspended` field diff --git a/spec/request/organizations_spec.rb b/spec/request/organizations_spec.rb index fc5b009d716..d2d2ff7a5f2 100644 --- a/spec/request/organizations_spec.rb +++ b/spec/request/organizations_spec.rb @@ -169,6 +169,36 @@ module VCAP::CloudController expect(last_response.status).to eq(201) end end + + context 'when a non-admin sets the suspended field' do + before do + VCAP::CloudController::FeatureFlag.make(name: 'user_org_creation', enabled: true) + end + + it 'returns 403 NotAuthorized for suspended: true' do + suspended_request_body = { name: 'sneaky-org', suspended: true }.to_json + + expect do + post '/v3/organizations', suspended_request_body, user_header + end.not_to change(Organization, :count) + + expect(last_response.status).to eq(403) + expect(last_response).to have_error_message('You are not authorized to perform the requested action') + end + + it 'allows suspended: false (the default) as a no-op' do + request_body = { name: 'noop-suspended-org', suspended: false }.to_json + + expect do + post '/v3/organizations', request_body, user_header + end.to change(Organization, :count).by 1 + + expect(last_response.status).to eq(201) + created_org = Organization.last + expect(created_org.name).to eq('noop-suspended-org') + expect(created_org).not_to be_suspended + end + end end describe 'GET /v3/organizations' do @@ -1307,6 +1337,57 @@ module VCAP::CloudController it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS end + + context 'when a OrgManager mutates the suspended field' do + let(:org_manager) { VCAP::CloudController::User.make } + let(:org_manager_header) { headers_for(org_manager) } + + before do + organization1.add_user(org_manager) + organization1.add_manager(org_manager) + end + + it 'returns 403 NotAuthorized when attempting to suspend an active org' do + patch "/v3/organizations/#{organization1.guid}", + { suspended: true }.to_json, + org_manager_header.merge('CONTENT_TYPE' => 'application/json') + + expect(last_response.status).to eq(403) + expect(last_response).to have_error_message('You are not authorized to perform the requested action') + expect(organization1.reload).not_to be_suspended + end + + it 'returns CF-OrgSuspended when attempting to un-suspend a suspended org' do + organization1.update(status: VCAP::CloudController::Organization::SUSPENDED) + + patch "/v3/organizations/#{organization1.guid}", + { suspended: false }.to_json, + org_manager_header.merge('CONTENT_TYPE' => 'application/json') + + expect(last_response.status).to eq(403) + expect(last_response).to have_error_message('The organization is suspended') + expect(organization1.reload).to be_suspended + end + + it 'allows benign updates that do not touch the suspended field' do + patch "/v3/organizations/#{organization1.guid}", + { name: 'renamed-by-manager' }.to_json, + org_manager_header.merge('CONTENT_TYPE' => 'application/json') + + expect(last_response.status).to eq(200) + expect(organization1.reload.name).to eq('renamed-by-manager') + end + + it 'allows a no-op suspended update (suspended matches current state)' do + patch "/v3/organizations/#{organization1.guid}", + { name: 'still-active', suspended: false }.to_json, + org_manager_header.merge('CONTENT_TYPE' => 'application/json') + + expect(last_response.status).to eq(200) + expect(organization1.reload.name).to eq('still-active') + expect(organization1.reload).not_to be_suspended + end + end end describe 'DELETE /v3/organizations/:guid' do diff --git a/spec/request/v2/organizations_spec.rb b/spec/request/v2/organizations_spec.rb index c09e59ddd64..c6e60763838 100644 --- a/spec/request/v2/organizations_spec.rb +++ b/spec/request/v2/organizations_spec.rb @@ -122,5 +122,83 @@ expect(decoded_response['description']).to eq('Current usage exceeds new quota values. This org currently contains apps running with an unlimited log rate limit.') end end + + context 'when a OrgManager mutates the status field' do + let(:org_manager) { VCAP::CloudController::User.make } + let(:org_manager_header) { headers_for(org_manager) } + let(:admin_header) { headers_for(user, scopes: %w[cloud_controller.admin]) } + + before do + org.add_user(org_manager) + org.add_manager(org_manager) + end + + it 'returns 403 NotAuthorized when attempting to suspend an active org' do + put "/v2/organizations/#{org.guid}", { status: 'suspended' }.to_json, org_manager_header + + expect(last_response).to have_status_code(403) + expect(decoded_response['error_code']).to eq('CF-NotAuthorized') + expect(org.reload).not_to be_suspended + end + + it 'returns 403 NotAuthorized when attempting to un-suspend a suspended org' do + org.update(status: VCAP::CloudController::Organization::SUSPENDED) + + put "/v2/organizations/#{org.guid}", { status: 'active' }.to_json, org_manager_header + + expect(last_response).to have_status_code(403) + expect(decoded_response['error_code']).to eq('CF-NotAuthorized') + expect(org.reload).to be_suspended + end + + it 'still allows an admin to mutate status' do + put "/v2/organizations/#{org.guid}", { status: 'suspended' }.to_json, admin_header + + expect(last_response).to have_status_code(201) + expect(org.reload).to be_suspended + end + + it 'allows a no-op status update (status matches current state) by an OrgManager' do + put "/v2/organizations/#{org.guid}", { status: 'active' }.to_json, org_manager_header + + expect(last_response).to have_status_code(201) + expect(org.reload).not_to be_suspended + end + end + end + + describe 'POST /v2/organizations' do + context 'when a non-admin sets status to suspended at create' do + let(:user_header) { headers_for(user) } + let(:admin_header) { headers_for(user, scopes: %w[cloud_controller.admin]) } + + before do + VCAP::CloudController::FeatureFlag.create(name: 'user_org_creation', enabled: true) + end + + it 'returns 403 NotAuthorized when a non-admin attempts to create a suspended org' do + post '/v2/organizations', { name: 'suspended-at-birth', status: 'suspended' }.to_json, user_header + + expect(last_response).to have_status_code(403) + expect(decoded_response['error_code']).to eq('CF-NotAuthorized') + expect(VCAP::CloudController::Organization.find(name: 'suspended-at-birth')).to be_nil + end + + it 'allows a non-admin to create an org with status: active (the default) as a no-op' do + post '/v2/organizations', { name: 'noop-active-org', status: 'active' }.to_json, user_header + + expect(last_response).to have_status_code(201) + created = VCAP::CloudController::Organization.find(name: 'noop-active-org') + expect(created).not_to be_nil + expect(created).not_to be_suspended + end + + it 'still allows an admin to create a suspended org' do + post '/v2/organizations', { name: 'admin-suspended-org', status: 'suspended' }.to_json, admin_header + + expect(last_response).to have_status_code(201) + expect(VCAP::CloudController::Organization.find(name: 'admin-suspended-org')).to be_suspended + end + end end end diff --git a/spec/unit/access/organization_access_spec.rb b/spec/unit/access/organization_access_spec.rb index f3be93f988a..195d6d1e032 100644 --- a/spec/unit/access/organization_access_spec.rb +++ b/spec/unit/access/organization_access_spec.rb @@ -114,6 +114,18 @@ module VCAP::CloudController end it_behaves_like('an access control', :create, flag_enabled_create_table) + + context 'status param is set to suspended' do + let(:op_params) { { 'status' => VCAP::CloudController::Organization::SUSPENDED } } + + it_behaves_like('an access control', :create, write_table) + end + + context 'status param is set to active (no-op)' do + let(:op_params) { { 'status' => VCAP::CloudController::Organization::ACTIVE } } + + it_behaves_like('an access control', :create, flag_enabled_create_table) + end end context 'when the flag is disabled' do @@ -141,6 +153,18 @@ module VCAP::CloudController it_behaves_like('an access control', :read_for_update, write_table) end + + context 'status param is set to suspended' do + let(:op_params) { { 'status' => VCAP::CloudController::Organization::SUSPENDED } } + + it_behaves_like('an access control', :read_for_update, write_table) + end + + context 'status param is set to active (no-op on an active org)' do + let(:op_params) { { 'status' => VCAP::CloudController::Organization::ACTIVE } } + + it_behaves_like('an access control', :read_for_update, update_table) + end end end