Restrict suspending orgs to admins (#5173)

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

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

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`

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

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

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

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