Restrict suspending orgs to admins (#5173) (#5181)

Author: johha

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 suspended_by_non_admin?(message)
+
     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 suspended_by_non_admin?(message)
+
     org = OrganizationUpdate.new(user_audit_info).update(org, message)
 
     render json: Presenters::V3::OrganizationPresenter.new(org), status: :ok
@@ -222,6 +227,10 @@ def isolation_segment_not_found!
     resource_not_found!(:isolation_segment)
   end
 
+  def suspended_by_non_admin?(message)
+    message.requested?(:suspended) && message.suspended && !permission_queryer.can_write_globally?
+  end
+
   def fetch_org(guid)
     Organization.where(guid:).first
   end

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,39 @@ module VCAP::CloudController
           expect(last_response.status).to eq(201)
         end
       end
+
+      context 'when a non-admin sets the suspended field' do
+        let(:org) { Organization.make }
+        let(:space) { Space.make(organization: org) }
+
+        before do
+          VCAP::CloudController::FeatureFlag.make(name: 'user_org_creation', enabled: true)
+        end
+
+        context 'with suspended: true' do
+          let(:api_call) do
+            ->(user_headers) { post '/v3/organizations', { name: "org-#{SecureRandom.uuid}", suspended: true }.to_json, user_headers }
+          end
+          let(:expected_codes_and_responses) do
+            h = Hash.new({ code: 403, errors: CF_NOT_AUTHORIZED }.freeze)
+            h['admin'] = { code: 201 }
+            h
+          end
+
+          it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS
+        end
+
+        context 'with suspended: false (no-op)' do
+          let(:api_call) do
+            ->(user_headers) { post '/v3/organizations', { name: "org-#{SecureRandom.uuid}", suspended: false }.to_json, user_headers }
+          end
+          let(:expected_codes_and_responses) do
+            Hash.new({ code: 201 }.freeze)
+          end
+
+          it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS
+        end
+      end
     end
 
     describe 'GET /v3/organizations' do
@@ -1307,6 +1340,59 @@ module VCAP::CloudController
 
         it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS
       end
+
+      context 'when a non-admin mutates the suspended field' do
+        let(:org) { Organization.make }
+        let(:space) { Space.make(organization: org) }
+
+        context 'on an active org with suspended: true' do
+          let(:api_call) do
+            ->(user_headers) { patch "/v3/organizations/#{org.guid}", { suspended: true }.to_json, user_headers.merge('CONTENT_TYPE' => 'application/json') }
+          end
+          let(:expected_codes_and_responses) do
+            h = Hash.new({ code: 403, errors: CF_NOT_AUTHORIZED }.freeze)
+            h['admin'] = { code: 200 }
+            h['no_role'] = { code: 404 }
+            h
+          end
+
+          it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS
+        end
+
+        context 'on a suspended org with suspended: false (un-suspend)' do
+          let(:api_call) do
+            ->(user_headers) { patch "/v3/organizations/#{org.guid}", { suspended: false }.to_json, user_headers.merge('CONTENT_TYPE' => 'application/json') }
+          end
+          let(:expected_codes_and_responses) do
+            h = Hash.new({ code: 403, errors: CF_NOT_AUTHORIZED }.freeze)
+            h['admin'] = { code: 200 }
+            h['org_manager'] = { code: 403, errors: CF_ORG_SUSPENDED }
+            h['no_role'] = { code: 404 }
+            h
+          end
+
+          before do
+            org.update(status: VCAP::CloudController::Organization::SUSPENDED)
+          end
+
+          it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS
+        end
+
+        context 'on an active org with suspended: false (no-op)' do
+          let(:api_call) do
+            ->(user_headers) { patch "/v3/organizations/#{org.guid}", { suspended: false }.to_json, user_headers.merge('CONTENT_TYPE' => 'application/json') }
+          end
+          let(:expected_codes_and_responses) do
+            h = Hash.new({ code: 403, errors: CF_NOT_AUTHORIZED }.freeze)
+            h['admin'] = { code: 200 }
+            h['org_manager'] = { code: 200 }
+            h['no_role'] = { code: 404 }
+            h
+          end
+
+          it_behaves_like 'permissions for single object endpoint', ALL_PERMISSIONS
+        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