diff --git a/app/controllers/admin/embeddable_hosts_controller.rb b/app/controllers/admin/embeddable_hosts_controller.rb index e70061f0c27..ee59743f920 100644 --- a/app/controllers/admin/embeddable_hosts_controller.rb +++ b/app/controllers/admin/embeddable_hosts_controller.rb @@ -1,23 +1,24 @@ class Admin::EmbeddableHostsController < Admin::AdminController def create - save_host(EmbeddableHost.new) + save_host(EmbeddableHost.new, :create) end def update host = EmbeddableHost.where(id: params[:id]).first - save_host(host) + save_host(host, :update) end def destroy host = EmbeddableHost.where(id: params[:id]).first host.destroy + StaffActionLogger.new(current_user).log_embeddable_host(host, UserHistory.actions[:embeddable_host_destroy]) render json: success_json end protected - def save_host(host) + def save_host(host, action) host.host = params[:embeddable_host][:host] host.path_whitelist = params[:embeddable_host][:path_whitelist] host.class_name = params[:embeddable_host][:class_name] @@ -25,6 +26,8 @@ class Admin::EmbeddableHostsController < Admin::AdminController host.category_id = SiteSetting.uncategorized_category_id if host.category_id.blank? if host.save + changes = host.saved_changes if action == :update + StaffActionLogger.new(current_user).log_embeddable_host(host, UserHistory.actions[:"embeddable_host_#{action}"], changes: changes) render_serialized(host, EmbeddableHostSerializer, root: 'embeddable_host', rest_serializer: true) else render_json_error(host) diff --git a/app/models/embeddable_host.rb b/app/models/embeddable_host.rb index c8cd6158cf6..84ce201cea8 100644 --- a/app/models/embeddable_host.rb +++ b/app/models/embeddable_host.rb @@ -3,6 +3,7 @@ require_dependency 'url_helper' class EmbeddableHost < ActiveRecord::Base validate :host_must_be_valid belongs_to :category + after_destroy :reset_embedding_settings before_validation do self.host.sub!(/^https?:\/\//, '') @@ -53,6 +54,12 @@ class EmbeddableHost < ActiveRecord::Base private + def reset_embedding_settings + unless EmbeddableHost.exists? + Embedding.settings.each { |s| SiteSetting.set(s.to_s, SiteSetting.defaults[s]) } + end + end + def host_must_be_valid if host !~ /\A[a-z0-9]+([\-\.]{1}[a-z0-9]+)*\.[a-z]{2,10}(:[0-9]{1,5})?(\/.*)?\Z/i && host !~ /\A(\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})(:[0-9]{1,5})?(\/.*)?\Z/ && diff --git a/app/models/user_history.rb b/app/models/user_history.rb index 7239e070e8d..be0f2fac7f9 100644 --- a/app/models/user_history.rb +++ b/app/models/user_history.rb @@ -88,7 +88,10 @@ class UserHistory < ActiveRecord::Base approve_user: 69, web_hook_create: 70, web_hook_update: 71, - web_hook_destroy: 72 + web_hook_destroy: 72, + embeddable_host_create: 73, + embeddable_host_update: 74, + embeddable_host_destroy: 75 ) end @@ -155,7 +158,10 @@ class UserHistory < ActiveRecord::Base :approve_user, :web_hook_create, :web_hook_update, - :web_hook_destroy + :web_hook_destroy, + :embeddable_host_create, + :embeddable_host_update, + :embeddable_host_destroy ] end diff --git a/app/services/staff_action_logger.rb b/app/services/staff_action_logger.rb index 05cfe8a0a39..b2e1e2f4aab 100644 --- a/app/services/staff_action_logger.rb +++ b/app/services/staff_action_logger.rb @@ -589,15 +589,7 @@ class StaffActionLogger "payload_url: #{web_hook.payload_url}" ] - if changes = opts[:changes] - changes.reject! { |k, v| k == "updated_at" } - old_values = [] - new_values = [] - changes.each do |k, v| - old_values << "#{k}: #{v[0]}" - new_values << "#{k}: #{v[1]}" - end - end + old_values, new_values = get_changes(opts[:changes]) UserHistory.create!(params(opts).merge( action: action, @@ -607,8 +599,33 @@ class StaffActionLogger )) end + def log_embeddable_host(embeddable_host, action, opts = {}) + old_values, new_values = get_changes(opts[:changes]) + + UserHistory.create!(params(opts).merge( + action: action, + context: "host: #{embeddable_host.host}", + previous_value: old_values&.join(", "), + new_value: new_values&.join(", ") + )) + end + private + def get_changes(changes) + return unless changes + + changes.delete("updated_at") + old_values = [] + new_values = [] + changes.each do |k, v| + old_values << "#{k}: #{v[0]}" + new_values << "#{k}: #{v[1]}" + end + + [old_values, new_values] + end + def params(opts = nil) opts ||= {} { acting_user_id: @admin.id, context: opts[:context] } diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index e971636a38b..e784eb8cb9c 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -3689,6 +3689,9 @@ en: web_hook_create: "webhook create" web_hook_update: "webhook update" web_hook_destroy: "webhook destroy" + embeddable_host_create: "embeddable host create" + embeddable_host_update: "embeddable host update" + embeddable_host_destroy: "embeddable host destroy" screened_emails: title: "Screened Emails" description: "When someone tries to create a new account, the following email addresses will be checked and the registration will be blocked, or some other action performed." diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 1762f56554c..7357e960ecf 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -4359,10 +4359,6 @@ en: unknown: "unknown" user_merged: "%{username} was merged into this account" user_delete_self: "Deleted by self from %{url}" - update: "Updated" - create: "Created" - destroy: "Destroyed" - reviewables: missing_version: "You must supply a version parameter" diff --git a/spec/models/embeddable_host_spec.rb b/spec/models/embeddable_host_spec.rb index 4d3f5fc9261..33d8b2ba35c 100644 --- a/spec/models/embeddable_host_spec.rb +++ b/spec/models/embeddable_host_spec.rb @@ -121,4 +121,26 @@ describe EmbeddableHost do end end + describe "reset_embedding_settings" do + it "resets all embedding related settings when last embeddable host is removed" do + host = Fabricate(:embeddable_host) + host2 = Fabricate(:embeddable_host) + + SiteSetting.embed_post_limit = 300 + SiteSetting.feed_polling_url = "http://test.com" + SiteSetting.feed_polling_enabled = true + + host2.destroy + + expect(SiteSetting.embed_post_limit).to eq(300) + expect(SiteSetting.feed_polling_url).to eq("http://test.com") + expect(SiteSetting.feed_polling_enabled).to eq(true) + + host.destroy + + expect(SiteSetting.embed_post_limit).to eq(SiteSetting.defaults[:embed_post_limit]) + expect(SiteSetting.feed_polling_url).to eq(SiteSetting.defaults[:feed_polling_url]) + expect(SiteSetting.feed_polling_enabled).to eq(SiteSetting.defaults[:feed_polling_enabled]) + end + end end diff --git a/spec/requests/admin/embeddable_hosts_controller_spec.rb b/spec/requests/admin/embeddable_hosts_controller_spec.rb index 403ed76e1e8..f0b2d654d45 100644 --- a/spec/requests/admin/embeddable_hosts_controller_spec.rb +++ b/spec/requests/admin/embeddable_hosts_controller_spec.rb @@ -4,4 +4,47 @@ describe Admin::EmbeddableHostsController do it "is a subclass of AdminController" do expect(Admin::EmbeddableHostsController < Admin::AdminController).to eq(true) end + + context 'while logged in as an admin' do + let(:admin) { Fabricate(:admin) } + let(:embeddable_host) { Fabricate(:embeddable_host) } + + before do + sign_in(admin) + end + + describe '#create' do + it "logs embeddable host create" do + post "/admin/embeddable_hosts.json", params: { + embeddable_host: { host: "test.com" } + } + + expect(response.status).to eq(200) + expect(UserHistory.where(acting_user_id: admin.id, + action: UserHistory.actions[:embeddable_host_create]).exists?).to eq(true) + end + end + + describe '#update' do + it "logs embeddable host update" do + put "/admin/embeddable_hosts/#{embeddable_host.id}.json", params: { + embeddable_host: { host: "test.com", class_name: "test-class", category_id: "3" } + } + + expect(response.status).to eq(200) + expect(UserHistory.where(acting_user_id: admin.id, + action: UserHistory.actions[:embeddable_host_update], + new_value: "host: test.com, class_name: test-class, category_id: 3").exists?).to eq(true) + end + end + + describe '#destroy' do + it "logs embeddable host destroy" do + delete "/admin/embeddable_hosts/#{embeddable_host.id}.json", params: {} + + expect(response.status).to eq(200) + expect(UserHistory.where(acting_user_id: admin.id, action: UserHistory.actions[:embeddable_host_destroy]).exists?).to eq(true) + end + end + end end