Add validations to admin settings (#10348)

* Add validations to admin settings

- Validate correct HTML markup
- Validate presence of contact username & e-mail
- Validate that all usernames are valid
- Validate that enums have expected values

* Fix code style issue

* Fix tests
pull/10356/head
Eugen Rochko 2019-03-23 14:07:04 +01:00 committed by GitHub
parent 55a9658ad8
commit 555c4e11ba
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 140 additions and 110 deletions

View File

@ -2,84 +2,29 @@
module Admin
class SettingsController < BaseController
ADMIN_SETTINGS = %w(
site_contact_username
site_contact_email
site_title
site_short_description
site_description
site_extended_description
site_terms
registrations_mode
closed_registrations_message
open_deletion
timeline_preview
show_staff_badge
bootstrap_timeline_accounts
theme
thumbnail
hero
mascot
min_invite_role
activity_api_enabled
peers_api_enabled
show_known_fediverse_at_about_page
preview_sensitive_media
custom_css
profile_directory
).freeze
BOOLEAN_SETTINGS = %w(
open_deletion
timeline_preview
show_staff_badge
activity_api_enabled
peers_api_enabled
show_known_fediverse_at_about_page
preview_sensitive_media
profile_directory
).freeze
UPLOAD_SETTINGS = %w(
thumbnail
hero
mascot
).freeze
def edit
authorize :settings, :show?
@admin_settings = Form::AdminSettings.new
end
def update
authorize :settings, :update?
settings_params.each do |key, value|
if UPLOAD_SETTINGS.include?(key)
upload = SiteUpload.where(var: key).first_or_initialize(var: key)
upload.update(file: value)
else
setting = Setting.where(var: key).first_or_initialize(var: key)
setting.update(value: value_for_update(key, value))
end
end
@admin_settings = Form::AdminSettings.new(settings_params)
flash[:notice] = I18n.t('generic.changes_saved_msg')
redirect_to edit_admin_settings_path
if @admin_settings.save
flash[:notice] = I18n.t('generic.changes_saved_msg')
redirect_to edit_admin_settings_path
else
render :edit
end
end
private
def settings_params
params.require(:form_admin_settings).permit(ADMIN_SETTINGS)
end
def value_for_update(key, value)
if BOOLEAN_SETTINGS.include?(key)
value == '1'
else
value
end
params.require(:form_admin_settings).permit(*Form::AdminSettings::KEYS)
end
end
end

View File

@ -3,49 +3,90 @@
class Form::AdminSettings
include ActiveModel::Model
delegate(
:site_contact_username,
:site_contact_username=,
:site_contact_email,
:site_contact_email=,
:site_title,
:site_title=,
:site_short_description,
:site_short_description=,
:site_description,
:site_description=,
:site_extended_description,
:site_extended_description=,
:site_terms,
:site_terms=,
:registrations_mode,
:registrations_mode=,
:closed_registrations_message,
:closed_registrations_message=,
:open_deletion,
:open_deletion=,
:timeline_preview,
:timeline_preview=,
:show_staff_badge,
:show_staff_badge=,
:bootstrap_timeline_accounts,
:bootstrap_timeline_accounts=,
:theme,
:theme=,
:min_invite_role,
:min_invite_role=,
:activity_api_enabled,
:activity_api_enabled=,
:peers_api_enabled,
:peers_api_enabled=,
:show_known_fediverse_at_about_page,
:show_known_fediverse_at_about_page=,
:preview_sensitive_media,
:preview_sensitive_media=,
:custom_css,
:custom_css=,
:profile_directory,
:profile_directory=,
to: Setting
)
KEYS = %i(
site_contact_username
site_contact_email
site_title
site_short_description
site_description
site_extended_description
site_terms
registrations_mode
closed_registrations_message
open_deletion
timeline_preview
show_staff_badge
bootstrap_timeline_accounts
theme
min_invite_role
activity_api_enabled
peers_api_enabled
show_known_fediverse_at_about_page
preview_sensitive_media
custom_css
profile_directory
).freeze
BOOLEAN_KEYS = %i(
open_deletion
timeline_preview
show_staff_badge
activity_api_enabled
peers_api_enabled
show_known_fediverse_at_about_page
preview_sensitive_media
profile_directory
).freeze
UPLOAD_KEYS = %i(
thumbnail
hero
mascot
).freeze
attr_accessor(*KEYS)
validates :site_short_description, :site_description, :site_extended_description, :site_terms, :closed_registrations_message, html: true
validates :registrations_mode, inclusion: { in: %w(open approved none) }
validates :min_invite_role, inclusion: { in: %w(disabled user moderator admin) }
validates :site_contact_email, :site_contact_username, presence: true
validates :site_contact_username, existing_username: true
validates :bootstrap_timeline_accounts, existing_username: { multiple: true }
def initialize(_attributes = {})
super
initialize_attributes
end
def save
return false unless valid?
KEYS.each do |key|
value = instance_variable_get("@#{key}")
if UPLOAD_KEYS.include?(key)
upload = SiteUpload.where(var: key).first_or_initialize(var: key)
upload.update(file: value)
else
setting = Setting.where(var: key).first_or_initialize(var: key)
setting.update(value: typecast_value(key, value))
end
end
end
private
def initialize_attributes
KEYS.each do |key|
instance_variable_set("@#{key}", Setting.public_send(key)) if instance_variable_get("@#{key}").nil?
end
end
def typecast_value(key, value)
if BOOLEAN_KEYS.include?(key)
value == '1'
else
value
end
end
end

View File

@ -0,0 +1,20 @@
# frozen_string_literal: true
class ExistingUsernameValidator < ActiveModel::EachValidator
def validate_each(record, attribute, value)
return if value.blank?
if options[:multiple]
missing_usernames = value.split(',').map { |username| username unless Account.find_local(username) }.compact
record.errors.add(attribute, I18n.t('existing_username_validator.not_found_multiple', usernames: missing_usernames.join(', '))) if missing_usernames.any?
else
record.errors.add(attribute, I18n.t('existing_username_validator.not_found')) unless Account.find_local(value)
end
end
private
def valid_html?(str)
Nokogiri::HTML.fragment(str).to_s == str
end
end

View File

@ -0,0 +1,14 @@
# frozen_string_literal: true
class HtmlValidator < ActiveModel::EachValidator
def validate_each(record, attribute, value)
return if value.blank?
record.errors.add(attribute, I18n.t('html_validator.invalid_markup')) unless valid_html?(value)
end
private
def valid_html?(str)
Nokogiri::HTML.fragment(str).to_s == str
end
end

View File

@ -2,6 +2,7 @@
= t('admin.settings.title')
= simple_form_for @admin_settings, url: admin_settings_path, html: { method: :patch } do |f|
= render 'shared/error_messages', object: @admin_settings
.fields-group
= f.input :site_title, wrapper: :with_label, label: t('admin.settings.site_title')

View File

@ -586,6 +586,9 @@ en:
content: We're sorry, but something went wrong on our end.
title: This page is not correct
noscript_html: To use the Mastodon web application, please enable JavaScript. Alternatively, try one of the <a href="%{apps_path}">native apps</a> for Mastodon for your platform.
existing_username_validator:
not_found: could not find a local user with that username
not_found_multiple: could not find %{usernames}
exports:
archive_takeout:
date: Date
@ -633,6 +636,8 @@ en:
validation_errors:
one: Something isn't quite right yet! Please review the error below
other: Something isn't quite right yet! Please review %{count} errors below
html_validator:
invalid_markup: contains invalid HTML markup
identity_proofs:
active: Active
authorize: Yes, authorize

View File

@ -37,7 +37,7 @@ SimpleNavigation::Configuration.run do |navigation|
primary.item :admin, safe_join([fa_icon('cogs fw'), t('admin.title')]), admin_dashboard_url, if: proc { current_user.staff? } do |admin|
admin.item :dashboard, safe_join([fa_icon('tachometer fw'), t('admin.dashboard.title')]), admin_dashboard_url
admin.item :settings, safe_join([fa_icon('cogs fw'), t('admin.settings.title')]), edit_admin_settings_url, if: -> { current_user.admin? }
admin.item :settings, safe_join([fa_icon('cogs fw'), t('admin.settings.title')]), edit_admin_settings_url, if: -> { current_user.admin? }, highlights_on: %r{/admin/settings}
admin.item :custom_emojis, safe_join([fa_icon('smile-o fw'), t('admin.custom_emojis.title')]), admin_custom_emojis_url, highlights_on: %r{/admin/custom_emojis}
admin.item :relays, safe_join([fa_icon('exchange fw'), t('admin.relays.title')]), admin_relays_url, if: -> { current_user.admin? }, highlights_on: %r{/admin/relays}
admin.item :subscriptions, safe_join([fa_icon('paper-plane-o fw'), t('admin.subscriptions.title')]), admin_subscriptions_url, if: -> { current_user.admin? }

View File

@ -19,6 +19,10 @@ RSpec.describe Admin::SettingsController, type: :controller do
end
describe 'PUT #update' do
before do
allow_any_instance_of(Form::AdminSettings).to receive(:valid?).and_return(true)
end
describe 'for a record that doesnt exist' do
around do |example|
before = Setting.site_extended_description