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.
This commit is contained in:
Justin DiRose 2020-11-13 09:57:49 -06:00 committed by GitHub
parent dc005c593e
commit 65e123498b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 65 additions and 37 deletions

View File

@ -28,6 +28,10 @@ export default Route.extend({
const fields = wrapper.model const fields = wrapper.model
.get("fields") .get("fields")
[wrapper.target].map((f) => f.name); [wrapper.target].map((f) => f.name);
if (wrapper.model.remote_theme) {
this.transitionTo("adminCustomizeThemes.index");
return;
}
if (!fields.includes(wrapper.field_name)) { if (!fields.includes(wrapper.field_name)) {
this.transitionTo( this.transitionTo(
"adminCustomizeThemes.edit", "adminCustomizeThemes.edit",

View File

@ -1,4 +1,5 @@
<div class="show-current-style"> <div class="show-current-style">
{{plugin-outlet name="admin-customize-themes-show-top" args=(hash theme=model)}}
<div class="title"> <div class="title">
{{#if editingName}} {{#if editingName}}
{{text-field value=model.name autofocus="true"}} {{text-field value=model.name autofocus="true"}}
@ -191,6 +192,7 @@
{{/d-section}} {{/d-section}}
{{/if}} {{/if}}
{{#unless model.remote_theme}}
<div class="control-unit"> <div class="control-unit">
<div class="mini-title">{{i18n "admin.customize.theme.css_html"}}</div> <div class="mini-title">{{i18n "admin.customize.theme.css_html"}}</div>
{{#if model.hasEditedFields}} {{#if model.hasEditedFields}}
@ -230,6 +232,7 @@
{{/if}} {{/if}}
{{d-button action=(action "addUploadModal") class="btn-default" icon="plus" label="admin.customize.theme.add"}} {{d-button action=(action "addUploadModal") class="btn-default" icon="plus" label="admin.customize.theme.add"}}
</div> </div>
{{/unless}}
{{#if extraFiles.length}} {{#if extraFiles.length}}
<div class="control-unit"> <div class="control-unit">

View File

@ -299,6 +299,10 @@ class Admin::ThemesController < Admin::AdminController
raise Discourse::InvalidAccess if !GlobalSetting.allowed_theme_ids.nil? raise Discourse::InvalidAccess if !GlobalSetting.allowed_theme_ids.nil?
end end
def ban_for_remote_theme!
raise Discourse::InvalidAccess if @theme.remote_theme
end
def add_relative_themes!(kind, ids) def add_relative_themes!(kind, ids)
expected = ids.map(&:to_i) expected = ids.map(&:to_i)
@ -357,6 +361,7 @@ class Admin::ThemesController < Admin::AdminController
return unless fields = theme_params[:theme_fields] return unless fields = theme_params[:theme_fields]
ban_in_allowlist_mode! ban_in_allowlist_mode!
ban_for_remote_theme!
fields.each do |field| fields.each do |field|
@theme.set_field( @theme.set_field(

View File

@ -370,6 +370,22 @@ describe Admin::ThemesController do
expect(UserHistory.where(action: UserHistory.actions[:change_theme]).count).to eq(1) expect(UserHistory.where(action: UserHistory.actions[:change_theme]).count).to eq(1)
end 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 it 'updates a child theme' do
child_theme = Fabricate(:theme, component: true) child_theme = Fabricate(:theme, component: true)
put "/admin/themes/#{child_theme.id}.json", params: { put "/admin/themes/#{child_theme.id}.json", params: {