FIX: restrict updates on `confirm_old_email` email templates

app/controllers/admin/email_templates_controller.rb

@@ -23,7 +23,7 @@ class Admin::EmailTemplatesController < Admin::AdminController
                      "system_messages.unsilenced", "system_messages.user_automatically_silenced",
                      "system_messages.welcome_invite", "system_messages.welcome_user", "test_mailer",
                      "user_notifications.account_created", "user_notifications.admin_login",
-                     "user_notifications.confirm_new_email", "user_notifications.confirm_old_email",
+                     "user_notifications.confirm_new_email",
                      "user_notifications.notify_old_email", "user_notifications.forgot_password",
                      "user_notifications.set_password", "user_notifications.signup",
                      "user_notifications.signup_after_approval",

app/controllers/admin/site_texts_controller.rb

@@ -7,6 +7,12 @@ class Admin::SiteTextsController < Admin::AdminController
      'login_required.welcome_message']
   end
 
+  def self.restricted_keys
+    ['user_notifications.confirm_old_email.title',
+     'user_notifications.confirm_old_email.subject_template',
+     'user_notifications.confirm_old_email.text_body_template']
+  end
+
   def index
     overridden = params[:overridden] == 'true'
     extras = {}
@@ -80,7 +86,7 @@ class Admin::SiteTextsController < Admin::AdminController
     end
 
     def find_site_text
-      raise Discourse::NotFound unless I18n.exists?(params[:id])
+      raise Discourse::NotFound unless I18n.exists?(params[:id]) && !self.class.restricted_keys.include?(params[:id])
       record_for(params[:id])
     end
 

db/migrate/20180519053933_delete_confirm_old_email_template_overrides.rb

@@ -0,0 +1,11 @@
+class DeleteConfirmOldEmailTemplateOverrides < ActiveRecord::Migration[5.1]
+  def up
+    execute "DELETE FROM translation_overrides WHERE translation_key = 'user_notifications.confirm_old_email.title'"
+    execute "DELETE FROM translation_overrides WHERE translation_key = 'user_notifications.confirm_old_email.subject_template'"
+    execute "DELETE FROM translation_overrides WHERE translation_key = 'user_notifications.confirm_old_email.text_body_template'"
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

spec/requests/admin/site_texts_controller_spec.rb

@@ -0,0 +1,74 @@
+require 'rails_helper'
+
+RSpec.describe Admin::SiteTextsController do
+  let(:admin) { Fabricate(:admin) }
+  let(:user) { Fabricate(:user) }
+  let(:headers) { { ACCEPT: 'application/json' } }
+
+  after do
+    TranslationOverride.delete_all
+    I18n.reload!
+  end
+
+  context "#update" do
+    it "raises an error if you aren't logged in" do
+      put '/admin/customize/site_texts/some_key', params: {
+        site_text: { value: 'foo' }
+      }, headers: headers
+      expect(response.status).to eq(404)
+    end
+
+    it "raises an error if you aren't an admin" do
+      sign_in(user)
+      put '/admin/customize/site_texts/some_key', params: {
+        site_text: { value: 'foo' }
+      }, headers: headers
+      expect(response.status).to eq(404)
+    end
+
+    context "when logged in as admin" do
+      before do
+        sign_in(admin)
+      end
+
+      it "returns 'not found' when an unknown key is used" do
+        put '/admin/customize/site_texts/some_key', params: {
+          site_text: { value: 'foo' }
+        }, headers: headers
+
+        expect(response).not_to be_success
+
+        json = ::JSON.parse(response.body)
+        expect(json['error_type']).to eq('not_found')
+      end
+
+      it "works as expectd with correct keys" do
+        put '/admin/customize/site_texts/title', params: {
+          site_text: { value: 'foo' }
+        }, headers: headers
+
+        expect(response).to be_success
+
+        json = ::JSON.parse(response.body)
+        expect(json).to be_present
+
+        site_text = json['site_text']
+        expect(site_text).to be_present
+
+        expect(site_text['id']).to eq('title')
+        expect(site_text['value']).to eq('foo')
+      end
+
+      it "does not update restricted keys" do
+        put '/admin/customize/site_texts/user_notifications.confirm_old_email.title', params: {
+          site_text: { value: 'foo' }
+        }, headers: headers
+
+        expect(response).not_to be_success
+
+        json = ::JSON.parse(response.body)
+        expect(json['error_type']).to eq('not_found')
+      end
+    end
+  end
+end