Merge pull request ManageIQ#275 from chrisarcand/settings-crud

Add Settings API for servers, zones, and regions
(cherry picked from commit 3f407638b474b1ff8b2945e9a13666d1fb061201)

https://bugzilla.redhat.com/show_bug.cgi?id=1535605

app/controllers/api/base_controller.rb

@@ -39,8 +39,47 @@ class BaseController < ActionController::API
     respond_to :json
     rescue_from_api_errors
 
+    def settings
+      id       = @req.collection_id
+      type     = @req.collection
+      klass    = collection_class(@req.collection)
+      resource = resource_search(id, type, klass)
+
+      case @req.method
+      when :patch
+        raise ForbiddenError, "You are not authorized to edit settings." unless super_admin?
+        resource.add_settings_for_resource(@req.json_body)
+      when :delete
+        raise ForbiddenError, "You are not authorized to remove settings." unless super_admin?
+        resource.remove_settings_path_for_resource(*@req.json_body)
+        head :no_content
+        return
+      end
+
+      if super_admin? || current_user.role_allows?(:identifier => 'ops_settings')
+        render :json => whitelist_settings(resource.settings_for_resource.to_hash)
+      else
+        raise ForbiddenError, "You are not authorized to view settings."
+      end
+    end
+
     private
 
+    def current_user
+      User.current_user
+    end
+
+    def super_admin?
+      current_user.super_admin_user?
+    end
+
+    def whitelist_settings(settings)
+      return settings if super_admin?
+
+      whitelisted_categories = ApiConfig.collections[:settings][:categories]
+      settings.with_indifferent_access.slice(*whitelisted_categories)
+    end
+
     def set_gettext_locale
       FastGettext.set_locale(LocaleResolver.resolve(User.current_user, headers))
     end

app/controllers/api/base_controller/parser.rb

@@ -22,7 +22,7 @@ def validate_api_request
         # Method Validation for the collection or sub-collection specified
         if cname && ctype
           mname = @req.method
-          unless collection_config.supports_http_method?(cname, mname) || mname == :options
+          unless collection_config.supports_http_method?(cname, mname) || ignore_http_method_validation?
             raise BadRequestError, "Unsupported HTTP Method #{mname} for the #{ctype} #{cname} specified"
           end
         end
@@ -37,7 +37,7 @@ def validate_optional_collection_classes
       end
 
       def validate_api_action
-        return unless @req.collection
+        return if @req.collection.blank? || ignore_http_method_validation?
         send("validate_#{@req.method}_method")
       end
 
@@ -173,6 +173,12 @@ def parse_fetch_tenant(data)
 
       private
 
+      def ignore_http_method_validation?
+        settings_request = @req.subcollection == 'settings' && collection_option?(:settings)
+
+        settings_request || @req.method == :options
+      end
+
       #
       # For Posts we need to support actions, let's validate those
       #
@@ -277,6 +283,7 @@ def validate_api_request_collection
       def validate_api_request_subcollection(cname, ctype)
         # Sub-Collection Validation for the specified Collection
         if cname && @req.subcollection
+          return [cname, ctype] if @req.subcollection == 'settings' && collection_option?(:settings)
           return [cname, ctype] if collection_option?(:arbitrary_resource_path)
           ctype = "Sub-Collection"
           unless collection_config.subcollection?(cname, @req.subcollection)

app/controllers/api/settings_controller.rb

@@ -14,6 +14,8 @@ def show
     private
 
     def whitelist_settings(settings)
+      return settings if super_admin?
+
       result_hash = {}
       ApiConfig.collections[:settings][:categories].each do |category_path|
         result_hash.deep_merge!(settings_entry_to_hash(category_path, entry_value(settings, category_path)))

config/api.yml

@@ -1440,6 +1440,7 @@
     :identifier: region
     :options:
     - :collection
+    - :settings
     :verbs: *gp
     :klass: MiqRegion
     :collection_actions:
@@ -1682,6 +1683,7 @@
     :description: EVM Servers
     :options:
     - :collection
+    - :settings
     :verbs: *g
     :klass: MiqServer
   :service_catalogs:
@@ -2474,6 +2476,7 @@
     :identifier: zone
     :options:
     - :collection
+    - :settings
     :verbs: *gp
     :klass: Zone
     :collection_actions:

config/routes.rb

@@ -64,6 +64,15 @@
             end
           end
         end
+
+        if collection.options.include?(:settings)
+          match(
+            "/:c_id/settings",
+            :to  => "#{collection_name}#settings",
+            :via => %w[get patch delete],
+            :as  => "#{collection_name.to_s.singularize}_settings",
+          )
+        end
       end
     end
   end

spec/requests/api/regions_spec.rb

@@ -40,4 +40,109 @@
       "id"   => region.id
     )
   end
+
+  describe "/api/regions/:id/settings" do
+    let(:region_number) { ApplicationRecord.my_region_number + 1 }
+    let(:id) { ApplicationRecord.id_in_region(1, region_number) }
+    let(:region) { FactoryGirl.create(:miq_region, :id => id, :region => region_number) }
+    let(:zone) { FactoryGirl.create(:zone, :id => id) }
+    let!(:server) { EvmSpecHelper.remote_miq_server(:id => id, :zone => zone) }
+    let(:original_timeout) { region.settings_for_resource[:api][:authentication_timeout] }
+    let(:super_admin) { FactoryGirl.create(:user, :role => 'super_administrator', :userid => 'alice', :password => 'alicepassword') }
+
+    it "shows the settings to an authenticated user with the proper role" do
+      api_basic_authorize(:ops_settings)
+
+      get(api_region_settings_url(nil, region))
+
+      expect(response).to have_http_status(:ok)
+    end
+
+    it "does not allow an authenticated user who doesn't have the proper role to view the settings" do
+      api_basic_authorize
+
+      get(api_region_settings_url(nil, region))
+
+      expect(response).to have_http_status(:forbidden)
+    end
+
+    it "does not allow an unauthenticated user to view the settings" do
+      get(api_region_settings_url(nil, region))
+
+      expect(response).to have_http_status(:unauthorized)
+    end
+
+    it "permits updates to settings for an authenticated super-admin user" do
+      api_basic_authorize(:user => super_admin.userid, :password => super_admin.password)
+
+      expect {
+        patch(api_region_settings_url(nil, region), :params => {:api => {:authentication_timeout => "1337.minutes"}})
+      }.to change { region.settings_for_resource[:api][:authentication_timeout] }.from(original_timeout).to("1337.minutes")
+
+      expect(response.parsed_body).to include("api" => a_hash_including("authentication_timeout" => "1337.minutes"))
+      expect(response).to have_http_status(:ok)
+    end
+
+    it "does not allow an authenticated non-super-admin user to update settings" do
+      api_basic_authorize
+
+      expect {
+        patch(api_region_settings_url(nil, region), :params => {:api => {:authentication_timeout => "10.minutes"}})
+      }.not_to change { region.settings_for_resource[:api][:authentication_timeout] }
+
+      expect(response).to have_http_status(:forbidden)
+    end
+
+    it "does not allow an unauthenticated user to update the settings" do
+      expect {
+        patch(api_region_settings_url(nil, region), :params => {:api => {:authentication_timeout => "10.minutes"}})
+      }.not_to change { region.settings_for_resource[:api][:authentication_timeout] }
+
+      expect(response).to have_http_status(:unauthorized)
+    end
+
+    context "with an existing settings change" do
+      before do
+        region.add_settings_for_resource("api" => {"authentication_timeout" => "7331.minutes"})
+      end
+
+      it "allows an authenticated super-admin user to delete settings" do
+        api_basic_authorize(:user => super_admin.userid, :password => super_admin.password)
+        expect(region.settings_for_resource["api"]["authentication_timeout"]).to eq("7331.minutes")
+
+        expect {
+          delete(
+            api_region_settings_url(nil, region),
+            :params => %i[api authentication_timeout].to_json # => hack because Rails will interpret these as query params in a DELETE
+          )
+        }.to change { region.settings_for_resource["api"]["authentication_timeout"] }.from("7331.minutes").to("30.seconds")
+
+        expect(response).to have_http_status(:no_content)
+      end
+
+      it "does not allow an authenticated non-super-admin user to delete settings" do
+        api_basic_authorize
+
+        expect {
+          delete(
+            api_region_settings_url(nil, region),
+            :params => %i[api authentication_timeout].to_json # => hack because Rails will interpret these as query params in a DELETE
+          )
+        }.not_to change { region.settings_for_resource["api"]["authentication_timeout"] }
+
+        expect(response).to have_http_status(:forbidden)
+      end
+
+      it "does not allow an unauthenticated user to delete settings`" do
+        expect {
+          delete(
+            api_region_settings_url(nil, region),
+            :params => %i[api authentication_timeout].to_json # => hack because Rails will interpret these as query params in a DELETE
+          )
+        }.not_to change { region.settings_for_resource["api"]["authentication_timeout"] }
+
+        expect(response).to have_http_status(:unauthorized)
+      end
+    end
+  end
 end

spec/requests/servers_spec.rb

@@ -0,0 +1,103 @@
+RSpec.describe "Servers" do
+  describe "/api/servers/:id/settings" do
+    let(:server) { FactoryGirl.create(:miq_server) }
+    let(:original_timeout) { server.settings_for_resource[:api][:authentication_timeout] }
+    let(:super_admin) { FactoryGirl.create(:user, :role => 'super_administrator', :userid => 'alice', :password => 'alicepassword') }
+
+
+    it "shows the settings to an authenticated user with the proper role" do
+      api_basic_authorize(:ops_settings)
+
+      get(api_server_settings_url(nil, server))
+
+      expect(response).to have_http_status(:ok)
+    end
+
+    it "does not allow an authenticated user who doesn't have the proper role to view the settings" do
+      api_basic_authorize
+
+      get(api_server_settings_url(nil, server))
+
+      expect(response).to have_http_status(:forbidden)
+    end
+
+    it "does not allow an unauthenticated user to view the settings" do
+      get(api_server_settings_url(nil, server))
+
+      expect(response).to have_http_status(:unauthorized)
+    end
+
+    it "permits updates to settings for an authenticated super-admin user" do
+      api_basic_authorize(:user => super_admin.userid, :password => super_admin.password)
+
+      expect {
+        patch(api_server_settings_url(nil, server), :params => {:api => {:authentication_timeout => "1337.minutes"}})
+      }.to change { server.settings_for_resource[:api][:authentication_timeout] }.from(original_timeout).to("1337.minutes")
+
+      expect(response.parsed_body).to include("api" => a_hash_including("authentication_timeout" => "1337.minutes"))
+      expect(response).to have_http_status(:ok)
+    end
+
+    it "does not allow an authenticated non-super-admin user to update settings" do
+      api_basic_authorize
+
+      expect {
+        patch(api_server_settings_url(nil, server), :params => {:api => {:authentication_timeout => "10.minutes"}})
+      }.not_to change { server.settings_for_resource[:api][:authentication_timeout] }
+
+      expect(response).to have_http_status(:forbidden)
+    end
+
+    it "does not allow an unauthenticated user to update the settings" do
+      expect {
+        patch(api_server_settings_url(nil, server), :params => {:api => {:authentication_timeout => "10.minutes"}})
+      }.not_to change { server.settings_for_resource[:api][:authentication_timeout] }
+
+      expect(response).to have_http_status(:unauthorized)
+    end
+
+    context "with an existing settings change" do
+      before do
+        server.add_settings_for_resource("api" => {"authentication_timeout" => "7331.minutes"})
+      end
+
+      it "allows an authenticated super-admin user to delete settings" do
+        api_basic_authorize(:user => super_admin.userid, :password => super_admin.password)
+        expect(server.settings_for_resource["api"]["authentication_timeout"]).to eq("7331.minutes")
+
+        expect {
+          delete(
+            api_server_settings_url(nil, server),
+            :params => %i[api authentication_timeout].to_json # => hack because Rails will interpret these as query params in a DELETE
+          )
+        }.to change { server.settings_for_resource["api"]["authentication_timeout"] }.from("7331.minutes").to("30.seconds")
+
+        expect(response).to have_http_status(:no_content)
+      end
+
+      it "does not allow an authenticated non-super-admin user to delete settings" do
+        api_basic_authorize
+
+        expect {
+          delete(
+            api_server_settings_url(nil, server),
+            :params => %i[api authentication_timeout].to_json # => hack because Rails will interpret these as query params in a DELETE
+          )
+        }.not_to change { server.settings_for_resource["api"]["authentication_timeout"] }
+
+        expect(response).to have_http_status(:forbidden)
+      end
+
+      it "does not allow an unauthenticated user to delete settings`" do
+        expect {
+          delete(
+            api_server_settings_url(nil, server),
+            :params => %i[api authentication_timeout].to_json # => hack because Rails will interpret these as query params in a DELETE
+          )
+        }.not_to change { server.settings_for_resource["api"]["authentication_timeout"] }
+
+        expect(response).to have_http_status(:unauthorized)
+      end
+    end
+  end
+end