diff --git a/app/assets/javascripts/admin/controllers/admin_logs_screened_ip_addresses_controller.js b/app/assets/javascripts/admin/controllers/admin_logs_screened_ip_addresses_controller.js index b733558aae3..303b7fb3434 100644 --- a/app/assets/javascripts/admin/controllers/admin_logs_screened_ip_addresses_controller.js +++ b/app/assets/javascripts/admin/controllers/admin_logs_screened_ip_addresses_controller.js @@ -9,6 +9,7 @@ Discourse.AdminLogsScreenedIpAddressesController = Ember.ArrayController.extend(Discourse.Presence, { loading: false, content: [], + itemController: 'adminLogsScreenedIpAddress', show: function() { var self = this; @@ -19,3 +20,72 @@ Discourse.AdminLogsScreenedIpAddressesController = Ember.ArrayController.extend( }); } }); + +Discourse.AdminLogsScreenedIpAddressController = Ember.ObjectController.extend({ + editing: false, + savedIpAddress: null, + + actions: { + allow: function(record) { + record.set('action', 'do_nothing'); + this.send('save', record); + }, + + block: function(record) { + record.set('action', 'block'); + this.send('save', record); + }, + + edit: function() { + if (!this.get('editing')) { + this.savedIpAddress = this.get('ip_address'); + } + this.set('editing', true); + }, + + cancel: function() { + if (this.get('savedIpAddress') && this.get('editing')) { + this.set('ip_address', this.get('savedIpAddress')); + } + this.set('editing', false); + }, + + save: function(record) { + var self = this; + var wasEditing = this.get('editing'); + this.set('editing', false); + record.save().then(function(saved){ + if (saved.success) { + self.set('savedIpAddress', null); + } else { + bootbox.alert(saved.errors); + if (wasEditing) self.set('editing', true); + } + }, function(e){ + if (e.responseJSON && e.responseJSON.errors) { + bootbox.alert(I18n.t("generic_error_with_reason", {error: e.responseJSON.errors.join('. ')})); + } else { + bootbox.alert(I18n.t("generic_error")); + } + if (wasEditing) self.set('editing', true); + }); + }, + + destroy: function(record) { + var self = this; + return bootbox.confirm(I18n.t("admin.logs.screened_ips.delete_confirm", {ip_address: record.get('ip_address')}), I18n.t("no_value"), I18n.t("yes_value"), function(result) { + if (result) { + record.destroy().then(function(deleted) { + if (deleted) { + self.get("parentController.content").removeObject(record); + } else { + bootbox.alert(I18n.t("generic_error")); + } + }, function(e){ + bootbox.alert(I18n.t("generic_error_with_reason", {error: "http: " + e.status + " - " + e.body})); + }); + } + }); + } + } +}); \ No newline at end of file diff --git a/app/assets/javascripts/admin/models/screened_ip_address.js b/app/assets/javascripts/admin/models/screened_ip_address.js index 1e81b61658e..8fe13582e13 100644 --- a/app/assets/javascripts/admin/models/screened_ip_address.js +++ b/app/assets/javascripts/admin/models/screened_ip_address.js @@ -8,14 +8,40 @@ @module Discourse **/ Discourse.ScreenedIpAddress = Discourse.Model.extend({ - // TODO: this is repeated in all 3 screened models. move it. actionName: function() { - if (this.get('action') === 'do_nothing') { - return I18n.t("admin.logs.screened_ips.allow"); + return I18n.t("admin.logs.screened_ips.actions." + this.get('action')); + }.property('action'), + + isBlocked: function() { + return (this.get('action') === 'block'); + }.property('action'), + + actionIcon: function() { + if (this.get('action') === 'block') { + return this.get('blockIcon'); } else { - return I18n.t("admin.logs.screened_actions." + this.get('action')); + return this.get('doNothingIcon'); } - }.property('action') + }.property('action'), + + blockIcon: function() { + return 'icon-remove'; + }.property(), + + doNothingIcon: function() { + return 'icon-ok'; + }.property(), + + save: function() { + return Discourse.ajax("/admin/logs/screened_ip_addresses/" + this.get('id') + ".json", { + type: 'PUT', + data: {ip_address: this.get('ip_address'), action_name: this.get('action')} + }); + }, + + destroy: function() { + return Discourse.ajax("/admin/logs/screened_ip_addresses/" + this.get('id') + ".json", {type: 'DELETE'}); + } }); Discourse.ScreenedIpAddress.reopenClass({ diff --git a/app/assets/javascripts/admin/templates/logs/screened_ip_addresses.js.handlebars b/app/assets/javascripts/admin/templates/logs/screened_ip_addresses.js.handlebars index ed6bc5521f4..b929daf340c 100644 --- a/app/assets/javascripts/admin/templates/logs/screened_ip_addresses.js.handlebars +++ b/app/assets/javascripts/admin/templates/logs/screened_ip_addresses.js.handlebars @@ -5,13 +5,14 @@ {{else}} {{#if model.length}} -
+
{{i18n admin.logs.ip_address}}
{{i18n admin.logs.action}}
{{i18n admin.logs.match_count}}
{{i18n admin.logs.last_match_at}}
{{i18n admin.logs.created_at}}
+
diff --git a/app/assets/javascripts/admin/templates/logs/screened_ip_addresses_list_item.js.handlebars b/app/assets/javascripts/admin/templates/logs/screened_ip_addresses_list_item.js.handlebars index 22c06f7e9d9..bd5c055d123 100644 --- a/app/assets/javascripts/admin/templates/logs/screened_ip_addresses_list_item.js.handlebars +++ b/app/assets/javascripts/admin/templates/logs/screened_ip_addresses_list_item.js.handlebars @@ -1,5 +1,14 @@ -
{{ip_address}}
-
{{actionName}}
+
+ {{#if editing}} + {{textField value=ip_address autofocus="autofocus"}} + {{else}} + {{ip_address}} + {{/if}} +
+
+ + {{actionName}} +
{{match_count}}
{{#if last_match_at}} @@ -7,4 +16,18 @@ {{/if}}
{{unboundAgeWithTooltip created_at}}
+
+ {{#unless editing}} + + + {{#if isBlocked}} + + {{else}} + + {{/if}} + {{else}} + + {{i18n cancel}} + {{/unless}} +
\ No newline at end of file diff --git a/app/assets/stylesheets/common/admin/admin_base.scss b/app/assets/stylesheets/common/admin/admin_base.scss index f6958d48f62..7f3b67c89c4 100644 --- a/app/assets/stylesheets/common/admin/admin_base.scss +++ b/app/assets/stylesheets/common/admin/admin_base.scss @@ -700,16 +700,44 @@ table { // Logs +.admin-logs-table { + input.ember-text-field { + padding: 1px 4px; + } +} + .screened-emails, .screened-urls, .screened-ip-addresses { width: 900px; .email, .url { width: 300px; } - .action, .match_count, .last_match_at, .created_at, .ip_address { + .action, .match_count, .last_match_at, .created_at { + text-align: center; + width: 110px; + } +} + +.screened-emails, .screened-urls { + .ip_address { width: 110px; text-align: center; } } +.screened-ip-addresses { + .ip_address { + width: 150px; + text-align: left; + input { + width: 130px; + } + } + .actions { + width: 275px; + a { + text-decoration: underline; + } + } +} .staff-actions { width: 100%; diff --git a/app/controllers/admin/screened_ip_addresses_controller.rb b/app/controllers/admin/screened_ip_addresses_controller.rb index 6c1da6d1dbb..3e0cc5aea7e 100644 --- a/app/controllers/admin/screened_ip_addresses_controller.rb +++ b/app/controllers/admin/screened_ip_addresses_controller.rb @@ -1,8 +1,34 @@ class Admin::ScreenedIpAddressesController < Admin::AdminController + before_filter :fetch_screened_ip_address, only: [:update, :destroy] + def index - screened_emails = ScreenedIpAddress.limit(200).order('last_match_at desc').to_a - render_serialized(screened_emails, ScreenedIpAddressSerializer) + screened_ip_addresses = ScreenedIpAddress.limit(200).order('last_match_at desc').to_a + render_serialized(screened_ip_addresses, ScreenedIpAddressSerializer) end + def update + if @screened_ip_address.update_attributes(allowed_params) + render json: success_json + else + render_json_error(@screened_ip_address) + end + end + + def destroy + @screened_ip_address.destroy + render json: success_json + end + + private + + def allowed_params + params.require(:ip_address) + params.permit(:ip_address, :action_name) + end + + def fetch_screened_ip_address + @screened_ip_address = ScreenedIpAddress.find(params[:id]) + end + end diff --git a/app/models/screened_ip_address.rb b/app/models/screened_ip_address.rb index 30d3cafe3c4..009e0efae4d 100644 --- a/app/models/screened_ip_address.rb +++ b/app/models/screened_ip_address.rb @@ -8,7 +8,7 @@ class ScreenedIpAddress < ActiveRecord::Base default_action :block - validates :ip_address, presence: true, uniqueness: true + validates :ip_address, ip_address_format: true, presence: true def self.watch(ip_address, opts={}) match_for_ip_address(ip_address) || create(opts.slice(:action_type).merge(ip_address: ip_address)) diff --git a/app/serializers/screened_ip_address_serializer.rb b/app/serializers/screened_ip_address_serializer.rb index 43dd91000d1..d9d9048f59f 100644 --- a/app/serializers/screened_ip_address_serializer.rb +++ b/app/serializers/screened_ip_address_serializer.rb @@ -1,5 +1,6 @@ class ScreenedIpAddressSerializer < ApplicationSerializer - attributes :ip_address, + attributes :id, + :ip_address, :action, :match_count, :last_match_at, diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index ac44464e76c..f07144cc12e 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1223,6 +1223,9 @@ en: last_match_at: "Last Matched" match_count: "Matches" ip_address: "IP" + delete: 'Delete' + edit: 'Edit' + save: 'Save' screened_actions: block: "block" do_nothing: "do nothing" @@ -1260,7 +1263,10 @@ en: screened_ips: title: "Screened IPs" description: "IP addresses that are being watched." - allow: "allow" + delete_confirm: "Are you sure you want to remove the rule for %{ip_address}?" + actions: + block: "Block" + do_nothing: "Allow" impersonate: title: "Impersonate User" diff --git a/config/routes.rb b/config/routes.rb index 57c0aff4edc..38e17c29492 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -67,7 +67,7 @@ Discourse::Application.routes.draw do scope '/logs' do resources :staff_action_logs, only: [:index] resources :screened_emails, only: [:index] - resources :screened_ip_addresses, only: [:index] + resources :screened_ip_addresses, only: [:index, :update, :destroy] resources :screened_urls, only: [:index] end diff --git a/lib/screening_model.rb b/lib/screening_model.rb index 5d9b86ad00c..9fb02f6a1f3 100644 --- a/lib/screening_model.rb +++ b/lib/screening_model.rb @@ -23,6 +23,11 @@ module ScreeningModel self.action_type ||= self.class.actions[self.class.df_action] end + def action_name=(arg) + raise ArgumentError.new("Invalid action type #{arg}") if arg.nil? or !self.class.actions.has_key?(arg.to_sym) + self.action_type = self.class.actions[arg.to_sym] + end + def record_match! self.match_count += 1 self.last_match_at = Time.zone.now diff --git a/lib/validators/ip_address_format_validator.rb b/lib/validators/ip_address_format_validator.rb new file mode 100644 index 00000000000..a08e2901444 --- /dev/null +++ b/lib/validators/ip_address_format_validator.rb @@ -0,0 +1,11 @@ +# Allows unique IP address (10.0.1.20), and IP addresses with a mask (10.0.0.0/8). +# Useful when storing in a Postgresql inet column. +class IpAddressFormatValidator < ActiveModel::EachValidator + + def validate_each(record, attribute, value) + unless record.ip_address.nil? or record.ip_address.split('/').first =~ Resolv::AddressRegex + record.errors.add(attribute, :invalid) + end + end + +end diff --git a/spec/components/validators/allowed_ip_address_validator_spec.rb b/spec/components/validators/allowed_ip_address_validator_spec.rb index 99c47fba613..3df87639a19 100644 --- a/spec/components/validators/allowed_ip_address_validator_spec.rb +++ b/spec/components/validators/allowed_ip_address_validator_spec.rb @@ -30,4 +30,5 @@ describe AllowedIpAddressValidator do record.errors[:ip_address].should_not be_present end end + end \ No newline at end of file diff --git a/spec/components/validators/ip_address_format_validator_spec.rb b/spec/components/validators/ip_address_format_validator_spec.rb new file mode 100644 index 00000000000..fa34e16e5ba --- /dev/null +++ b/spec/components/validators/ip_address_format_validator_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe IpAddressFormatValidator do + + let(:record) { Fabricate.build(:screened_ip_address, ip_address: '99.232.23.123') } + let(:validator) { described_class.new({attributes: :ip_address}) } + subject(:validate) { validator.validate_each(record, :ip_address, record.ip_address) } + + [nil, '99.232.23.123', '99.232.0.0/16', 'fd12:db8::ff00:42:8329', 'fc00::/7'].each do |arg| + it "should not add an error for #{arg}" do + record.ip_address = arg + validate + record.errors[:ip_address].should_not be_present + end + end + + it 'should add an error for invalid IP address' do + record.ip_address = '99.99.99' + validate + record.errors[:ip_address].should be_present + end +end diff --git a/spec/controllers/admin/screened_ip_addresses_controller_spec.rb b/spec/controllers/admin/screened_ip_addresses_controller_spec.rb index a0100a19b62..2d78a00e674 100644 --- a/spec/controllers/admin/screened_ip_addresses_controller_spec.rb +++ b/spec/controllers/admin/screened_ip_addresses_controller_spec.rb @@ -7,7 +7,7 @@ describe Admin::ScreenedIpAddressesController do let!(:user) { log_in(:admin) } - context '.index' do + describe 'index' do before do xhr :get, :index end diff --git a/spec/models/screened_ip_address_spec.rb b/spec/models/screened_ip_address_spec.rb index e973dd0f3fd..b59d0ebe31e 100644 --- a/spec/models/screened_ip_address_spec.rb +++ b/spec/models/screened_ip_address_spec.rb @@ -8,6 +8,29 @@ describe ScreenedIpAddress do it 'sets a default action_type' do described_class.create(valid_params).action_type.should == described_class.actions[:block] end + + it 'sets an error when ip_address is invalid' do + described_class.create(valid_params.merge(ip_address: '99.99.99')).errors[:ip_address].should be_present + end + + it 'can set action_type using the action_name virtual attribute' do + described_class.new(valid_params.merge(action_name: :do_nothing)).action_type.should == described_class.actions[:do_nothing] + described_class.new(valid_params.merge(action_name: :block)).action_type.should == described_class.actions[:block] + described_class.new(valid_params.merge(action_name: 'do_nothing')).action_type.should == described_class.actions[:do_nothing] + described_class.new(valid_params.merge(action_name: 'block')).action_type.should == described_class.actions[:block] + end + + it 'raises a useful exception when action is invalid' do + expect { + described_class.new(valid_params.merge(action_name: 'dance')) + }.to raise_error(ArgumentError) + end + + it 'raises a useful exception when action is nil' do + expect { + described_class.new(valid_params.merge(action_name: nil)) + }.to raise_error(ArgumentError) + end end describe '#watch' do