From 65e123498b4f8f260a8461e48995a3ca166b605e Mon Sep 17 00:00:00 2001 From: Justin DiRose Date: Fri, 13 Nov 2020 09:57:49 -0600 Subject: [PATCH] FIX: Disallow editing of remote themes (#11189) Allowing the editing of remote themes has been something Discourse has advised against for some time. This commit removes the ability to edit or upload files to remote themes from Admin > Customize to enforce the recommended practice. --- .../routes/admin-customize-themes-edit.js | 4 + .../addon/templates/customize-themes-show.hbs | 77 ++++++++++--------- app/controllers/admin/themes_controller.rb | 5 ++ spec/requests/admin/themes_controller_spec.rb | 16 ++++ 4 files changed, 65 insertions(+), 37 deletions(-) diff --git a/app/assets/javascripts/admin/addon/routes/admin-customize-themes-edit.js b/app/assets/javascripts/admin/addon/routes/admin-customize-themes-edit.js index 1576fac0ec4..030645e445a 100644 --- a/app/assets/javascripts/admin/addon/routes/admin-customize-themes-edit.js +++ b/app/assets/javascripts/admin/addon/routes/admin-customize-themes-edit.js @@ -28,6 +28,10 @@ export default Route.extend({ const fields = wrapper.model .get("fields") [wrapper.target].map((f) => f.name); + if (wrapper.model.remote_theme) { + this.transitionTo("adminCustomizeThemes.index"); + return; + } if (!fields.includes(wrapper.field_name)) { this.transitionTo( "adminCustomizeThemes.edit", diff --git a/app/assets/javascripts/admin/addon/templates/customize-themes-show.hbs b/app/assets/javascripts/admin/addon/templates/customize-themes-show.hbs index 4bbdb6651cf..4bd259e07c6 100644 --- a/app/assets/javascripts/admin/addon/templates/customize-themes-show.hbs +++ b/app/assets/javascripts/admin/addon/templates/customize-themes-show.hbs @@ -1,4 +1,5 @@
+ {{plugin-outlet name="admin-customize-themes-show-top" args=(hash theme=model)}}
{{#if editingName}} {{text-field value=model.name autofocus="true"}} @@ -191,45 +192,47 @@ {{/d-section}} {{/if}} -
-
{{i18n "admin.customize.theme.css_html"}}
- {{#if model.hasEditedFields}} -
{{i18n "admin.customize.theme.custom_sections"}}
-
    - {{#each editedFieldsFormatted as |field|}} -
  • {{field}}
    • - {{/each}} -
- {{else}} -
- {{i18n "admin.customize.theme.edit_css_html_help"}} -
- {{/if}} + {{#unless model.remote_theme}} +
+
{{i18n "admin.customize.theme.css_html"}}
+ {{#if model.hasEditedFields}} +
{{i18n "admin.customize.theme.custom_sections"}}
+
    + {{#each editedFieldsFormatted as |field|}} +
  • {{field}}
    • + {{/each}} +
+ {{else}} +
+ {{i18n "admin.customize.theme.edit_css_html_help"}} +
+ {{/if}} - {{d-button - class="btn-default edit" - action=(action "editTheme") - label="admin.customize.theme.edit_css_html"}} -
+ {{d-button + class="btn-default edit" + action=(action "editTheme") + label="admin.customize.theme.edit_css_html"}} +
-
-
{{i18n "admin.customize.theme.uploads"}}
- {{#if model.uploads}} -
    - {{#each model.uploads as |upload|}} -
  • - ${{upload.name}}: {{upload.filename}} - - {{d-button action=(action "removeUpload") actionParam=upload class="second btn-default btn-default cancel-edit" icon="times"}} - -
    • - {{/each}} -
- {{else}} -
{{i18n "admin.customize.theme.no_uploads"}}
- {{/if}} - {{d-button action=(action "addUploadModal") class="btn-default" icon="plus" label="admin.customize.theme.add"}} -
+
+
{{i18n "admin.customize.theme.uploads"}}
+ {{#if model.uploads}} +
    + {{#each model.uploads as |upload|}} +
  • + ${{upload.name}}: {{upload.filename}} + + {{d-button action=(action "removeUpload") actionParam=upload class="second btn-default btn-default cancel-edit" icon="times"}} + +
    • + {{/each}} +
+ {{else}} +
{{i18n "admin.customize.theme.no_uploads"}}
+ {{/if}} + {{d-button action=(action "addUploadModal") class="btn-default" icon="plus" label="admin.customize.theme.add"}} +
+ {{/unless}} {{#if extraFiles.length}}
diff --git a/app/controllers/admin/themes_controller.rb b/app/controllers/admin/themes_controller.rb index b4c222dd0f0..7ddb3e49a0a 100644 --- a/app/controllers/admin/themes_controller.rb +++ b/app/controllers/admin/themes_controller.rb @@ -299,6 +299,10 @@ class Admin::ThemesController < Admin::AdminController raise Discourse::InvalidAccess if !GlobalSetting.allowed_theme_ids.nil? end + def ban_for_remote_theme! + raise Discourse::InvalidAccess if @theme.remote_theme + end + def add_relative_themes!(kind, ids) expected = ids.map(&:to_i) @@ -357,6 +361,7 @@ class Admin::ThemesController < Admin::AdminController return unless fields = theme_params[:theme_fields] ban_in_allowlist_mode! + ban_for_remote_theme! fields.each do |field| @theme.set_field( diff --git a/spec/requests/admin/themes_controller_spec.rb b/spec/requests/admin/themes_controller_spec.rb index f75c1c7ae20..43b5c9b12b6 100644 --- a/spec/requests/admin/themes_controller_spec.rb +++ b/spec/requests/admin/themes_controller_spec.rb @@ -370,6 +370,22 @@ describe Admin::ThemesController do expect(UserHistory.where(action: UserHistory.actions[:change_theme]).count).to eq(1) end + it 'blocks remote theme fields from being locally edited' do + r = RemoteTheme.create!(remote_url: "https://magic.com/repo.git") + theme.update!(remote_theme_id: r.id) + + put "/admin/themes/#{theme.id}.json", params: { + theme: { + theme_fields: [ + { name: 'scss', target: 'common', value: '' }, + { name: 'test', target: 'common', value: 'filename.jpg', upload_id: 4 } + ] + } + } + + expect(response.status).to eq(403) + end + it 'updates a child theme' do child_theme = Fabricate(:theme, component: true) put "/admin/themes/#{child_theme.id}.json", params: {